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

2001-06-27 Thread Roman Zippel

Hi,

Martin Wilck wrote:

> Hum - is there no simple way to determine whether a pointer is
> a valid pointer to something returned by __get_free_pages ()? You are
> right, S390 in particular seems to allow arbitrary addresses starting from
> 0.

M68k does so too, although the first page is never used and usually
unmapped to catch NULL pointers.

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Martin Wilck


> PAGE_OFFSET definitely works for me, but a quick scan of the headers
> suggests that non-sun3 m68k builds define PAGE_OFFSET as 0, as does
> s390.

Hum - is there no simple way to determine whether a pointer is
a valid pointer to something returned by __get_free_pages ()? You are
right, S390 in particular seems to allow arbitrary addresses starting from
0.

> Sure, the overloading is self-admittedly hacky, but (again I assume)
> the motivation was to avoid breaking the clients, many of which are
> not in the kernel.org tree. Your proposed change overloads a third
> interpretation on start, namely an arbitrary pointer, outside the
> page allocation.

For some reason I was convinced that this was the originally intended
use of start. The only quotes I find right now are Ori Pomerantz'
Module Programming Guide (http://www.linuxdoc.org/LDP/lkmpg/node16.html)
and Rubini's "Writing Device Drivers", chapter 4. Also, the comment in the
code
if (!start) {
/*
 * For proc files that are less than 4k
 */
...
}
supports this notion somehow (start only set if data size > page size).

After all, unless you want to mangle the file position as intended by
the hack, there is no point in touching start at all in proc_read (),
ppos will be updated automatically.

Perhaps I have misunderstood something here.
Who wrote the original code, after all?

Martin

-- 
Martin Wilck <[EMAIL PROTECTED]>
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Jonathan Lundell

At 10:07 AM +0200 2001-06-27, Martin Wilck wrote:
>On Tue, 26 Jun 2001, Jonathan Lundell wrote:
>
>>  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.
>
>Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
>below).
>Unless I'm mislead, legitimate values of "start" as a pointer are always
>larger than that, and I can hardly imagin e a case where the "unsigned
>int" value of start must be greater than PAGE_OFFSET.


PAGE_OFFSET definitely works for me, but a quick scan of the headers 
suggests that non-sun3 m68k builds define PAGE_OFFSET as 0, as does 
s390.

Maybe you want max(PAGE_SIZE, PAGE_OFFSET).

>I insist that relying on the comparison of two pointers is the wrong
>thing. If (as you suggest) the major use of "start" has migrated from the
>original intention to that of the "hack", this should be reflected
>in the interface by making the "start" parameter to read_proc ()
>an unsigned long. Everything else is misleading and error-prone.
>For now, "start" is a char* and should be treated as such.

That's the hack, though. Rusty should chime in, but the implicit 
restriction on start in the original hack (by the time we get to the 
test we're talking about) is that it's either a pointer of the form 
page+offset, where offset < PAGE_SIZE, or it's a (relatively) small 
file offset.

That's a reasonable assumption given that the procedure is 
dynamically allocating page. After all, why would you allocate the 
buffer and then not use it?

Sure, the overloading is self-admittedly hacky, but (again I assume) 
the motivation was to avoid breaking the clients, many of which are 
not in the kernel.org tree. Your proposed change overloads a third 
interpretation on start, namely an arbitrary pointer, outside the 
page allocation.

>  > 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.
>
>That's right, but nevertheless (repeat) comparing "start" and "page" is
>wrong.

Not given the implied restriction that, if start is a pointer at all, 
it's a pointer within page's allocation. And after all, PAGE_OFFSET 
is effectively a pointer.
-- 
/Jonathan Lundell.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Martin Wilck

On Tue, 26 Jun 2001, Jonathan Lundell wrote:

> 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.

Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
below).

Unless I'm mislead, legitimate values of "start" as a pointer are always
larger than that, and I can hardly imagin e a case where the "unsigned
int" value of start must be greater than PAGE_OFFSET.

I insist that relying on the comparison of two pointers is the wrong
thing. If (as you suggest) the major use of "start" has migrated from the
original intention to that of the "hack", this should be reflected
in the interface by making the "start" parameter to read_proc ()
an unsigned long. Everything else is misleading and error-prone.
For now, "start" is a char* and should be treated as such.

> 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.

That's right, but nevertheless (repeat) comparing "start" and "page" is
wrong.

Regards,
Martin

-- 
Martin Wilck <[EMAIL PROTECTED]>
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113

--- linux-2.4.5/fs/proc/generic.c   Mon Jun 25 13:46:26 2001
+++ 2.4.5mw/fs/proc/generic.c   Wed Jun 27 11:22:14 2001
@@ -104,14 +104,14 @@
 * return the bytes, and set `start' to the desired offset
 * as an unsigned int. - [EMAIL PROTECTED]
 */
-   n -= copy_to_user(buf, start < page ? page : start, n);
+   n -= copy_to_user(buf, (unsigned long) start < PAGE_OFFSET ? 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_OFFSET ? (unsigned long) start : 
+n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;



-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Martin Wilck

On Tue, 26 Jun 2001, Jonathan Lundell wrote:

 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.

Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
below).

Unless I'm mislead, legitimate values of start as a pointer are always
larger than that, and I can hardly imagin e a case where the unsigned
int value of start must be greater than PAGE_OFFSET.

I insist that relying on the comparison of two pointers is the wrong
thing. If (as you suggest) the major use of start has migrated from the
original intention to that of the hack, this should be reflected
in the interface by making the start parameter to read_proc ()
an unsigned long. Everything else is misleading and error-prone.
For now, start is a char* and should be treated as such.

 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.

That's right, but nevertheless (repeat) comparing start and page is
wrong.

Regards,
Martin

-- 
Martin Wilck [EMAIL PROTECTED]
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113

--- linux-2.4.5/fs/proc/generic.c   Mon Jun 25 13:46:26 2001
+++ 2.4.5mw/fs/proc/generic.c   Wed Jun 27 11:22:14 2001
@@ -104,14 +104,14 @@
 * return the bytes, and set `start' to the desired offset
 * as an unsigned int. - [EMAIL PROTECTED]
 */
-   n -= copy_to_user(buf, start  page ? page : start, n);
+   n -= copy_to_user(buf, (unsigned long) start  PAGE_OFFSET ? 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_OFFSET ? (unsigned long) start : 
+n; /* Move down the file */
nbytes -= n;
buf += n;
retval += n;



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Jonathan Lundell

At 10:07 AM +0200 2001-06-27, Martin Wilck wrote:
On Tue, 26 Jun 2001, Jonathan Lundell wrote:

  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.

Ok, let's use PAGE_OFFSET instead of PAGE_SIZE, then (see new patch
below).
Unless I'm mislead, legitimate values of start as a pointer are always
larger than that, and I can hardly imagin e a case where the unsigned
int value of start must be greater than PAGE_OFFSET.


PAGE_OFFSET definitely works for me, but a quick scan of the headers 
suggests that non-sun3 m68k builds define PAGE_OFFSET as 0, as does 
s390.

Maybe you want max(PAGE_SIZE, PAGE_OFFSET).

I insist that relying on the comparison of two pointers is the wrong
thing. If (as you suggest) the major use of start has migrated from the
original intention to that of the hack, this should be reflected
in the interface by making the start parameter to read_proc ()
an unsigned long. Everything else is misleading and error-prone.
For now, start is a char* and should be treated as such.

That's the hack, though. Rusty should chime in, but the implicit 
restriction on start in the original hack (by the time we get to the 
test we're talking about) is that it's either a pointer of the form 
page+offset, where offset  PAGE_SIZE, or it's a (relatively) small 
file offset.

That's a reasonable assumption given that the procedure is 
dynamically allocating page. After all, why would you allocate the 
buffer and then not use it?

Sure, the overloading is self-admittedly hacky, but (again I assume) 
the motivation was to avoid breaking the clients, many of which are 
not in the kernel.org tree. Your proposed change overloads a third 
interpretation on start, namely an arbitrary pointer, outside the 
page allocation.

   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.

That's right, but nevertheless (repeat) comparing start and page is
wrong.

Not given the implied restriction that, if start is a pointer at all, 
it's a pointer within page's allocation. And after all, PAGE_OFFSET 
is effectively a pointer.
-- 
/Jonathan Lundell.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Martin Wilck


 PAGE_OFFSET definitely works for me, but a quick scan of the headers
 suggests that non-sun3 m68k builds define PAGE_OFFSET as 0, as does
 s390.

Hum - is there no simple way to determine whether a pointer is
a valid pointer to something returned by __get_free_pages ()? You are
right, S390 in particular seems to allow arbitrary addresses starting from
0.

 Sure, the overloading is self-admittedly hacky, but (again I assume)
 the motivation was to avoid breaking the clients, many of which are
 not in the kernel.org tree. Your proposed change overloads a third
 interpretation on start, namely an arbitrary pointer, outside the
 page allocation.

For some reason I was convinced that this was the originally intended
use of start. The only quotes I find right now are Ori Pomerantz'
Module Programming Guide (http://www.linuxdoc.org/LDP/lkmpg/node16.html)
and Rubini's Writing Device Drivers, chapter 4. Also, the comment in the
code
if (!start) {
/*
 * For proc files that are less than 4k
 */
...
}
supports this notion somehow (start only set if data size  page size).

After all, unless you want to mangle the file position as intended by
the hack, there is no point in touching start at all in proc_read (),
ppos will be updated automatically.

Perhaps I have misunderstood something here.
Who wrote the original code, after all?

Martin

-- 
Martin Wilck [EMAIL PROTECTED]
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113



-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-27 Thread Roman Zippel

Hi,

Martin Wilck wrote:

 Hum - is there no simple way to determine whether a pointer is
 a valid pointer to something returned by __get_free_pages ()? You are
 right, S390 in particular seems to allow arbitrary addresses starting from
 0.

M68k does so too, although the first page is never used and usually
unmapped to catch NULL pointers.

bye, Roman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread Mike Galbraith

On Tue, 26 Jun 2001, 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.

It's dead simple.  My variable length data often didn't quite fit into
the transport vehicle.. so I whacked the excess to avoid the fault.

-Mike

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread 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 <[EMAIL PROTECTED]>
>FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113
>
>
>--- 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. - [EMAIL PROTECTED]
>*/
>-  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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread Martin Wilck

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.

Cheers,
Martin

-- 
Martin Wilck <[EMAIL PROTECTED]>
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113


--- 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. - [EMAIL PROTECTED]
 */
-   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;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread Martin Wilck

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.

Cheers,
Martin

-- 
Martin Wilck [EMAIL PROTECTED]
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113


--- 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. - [EMAIL PROTECTED]
 */
-   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;

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread 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 [EMAIL PROTECTED]
FSC EP PS DS1, Paderborn  Tel. +49 5251 8 15113


--- 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. - [EMAIL PROTECTED]
*/
-  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.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



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

2001-06-26 Thread Mike Galbraith

On Tue, 26 Jun 2001, 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.

It's dead simple.  My variable length data often didn't quite fit into
the transport vehicle.. so I whacked the excess to avoid the fault.

-Mike

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/