Re: [PATCH] proc_file_read() (Was: Re: proc_file_read() question)

[Jonathan Lundell]

At 7:14 PM +0200 2001-06-26, Martin Wilck wrote:
>Hi,
>
>>  Shhh ;-)  Last time that hack was mentioned, someone wanted to _remove_
>>  it.  It's a very nice little hack to have around, and IKD uses it.
>
>I am not saying it should be removed. But IMO it is a legitimate (if
>not the originally intended) use of "start" to serve as a pointer to
>a memory area allocated in the proc_read () function. This use is broken
>with this hack in its current form, because reading from such a file
>will fail depending on the (random) order of the page and start pointers.
>
>If I understand the "hack" right, legitimate offsets generated for it
>are always between 0 and PAGE_SIZE. Therefore the patch below would
>not break it, while overcoming the abovementioned problem, because
>legitimate page pointers will never be < PAGE_SIZE.
>
>Please correct me if I'm wrong.

I use the hack myself, to implement a record-oriented file where the 
file position is a record number. I could probably live with 
PAGE_SIZE, but the current hack works fine with start bigger than 
that, and it's possible that someone counts on it.

But if you're allocating your own buffer, you'd probably be better 
off writing your own file ops, and not using the default 
proc_file_read() at all. At the very least you'd save a redundant 
__get_free_page/free_page pair.

>Cheers,
>Martin
>
>--
>Martin Wilck     <[e-mail]>
>FSC EP PS DS1, Paderborn      Tel. [phone]
>
>
>--- linux-2.4.5/fs/proc/generic.c	Mon Jun 25 13:46:26 2001
>+++ 2.4.5mw/fs/proc/generic.c	Tue Jun 26 20:42:22 2001
>@@ -104,14 +104,14 @@
>  		 * return the bytes, and set `start' to the desired offset
>  		 * as an unsigned int. - [e-mail]
>  		 */
>-		n -= copy_to_user(buf, start < page ? page : start, n);
>+		n -= copy_to_user(buf, (unsigned long) start < 
>PAGE_SIZE ? page : start, n);
>  		if (n == 0) {
>  			if (retval == 0)
>  				retval = -EFAULT;
>  			break;
>  		}
>
>-		*ppos += start < page ? (long)start : n; /* Move down 
>the file */
>+		*ppos += (unsigned long) start < PAGE_SIZE ? 
>(unsigned long) start : n; /* Move down the file */
>  		nbytes -= n;
>  		buf += n;
>  		retval += n;

-- 
/Jonathan Lundell.