Messages in this thread Patch in this message | | | Date | Sat, 30 Aug 2008 23:44:20 +0400 | From | Cyrill Gorcunov <> | Subject | Re: buffer overflow in /proc/sys/sunrpc/transports |
| |
[Vegard Nossum - Sat, Aug 30, 2008 at 09:34:21PM +0200] | On Sat, Aug 30, 2008 at 9:21 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote: | > | Hm. I think this is wrong. Shouldn't we copy as many bytes as the user | > | indicated? | > | > Well, hard to say what user-space programmer is expecting from us. | > I mean - maybe he (reader) wants only part of results not the whole | > contents BUT by this way he never know what the whole conetnts would be | > until trying to read more (ie to check if there no more data from | > kernel side). What is preferred behaviour - i don't know :) | | For any other file, read(1) + read(1) should be exactly equivalent to | a read(2). What's the difference here?
Convinced completely :) Moreover proc_dodebug() does exactly the same as you talking about.
| | (Btw, thanks for the quick reply :-))
with my pleasure :)
| | | Vegard | | -- | "The animistic metaphor of the bug that maliciously sneaked in while | the programmer was not looking is intellectually dishonest as it | disguises that the error is the programmer's own creation." | -- E. W. Dijkstra, EWD1036 |
- Cyrill - ---
Index: linux-2.6.git/net/sunrpc/sysctl.c =================================================================== --- linux-2.6.git.orig/net/sunrpc/sysctl.c 2008-07-20 11:40:14.000000000 +0400 +++ linux-2.6.git/net/sunrpc/sysctl.c 2008-08-30 23:43:14.000000000 +0400 @@ -71,7 +71,8 @@ static int proc_do_xprt(ctl_table *table len = svc_print_xprts(tmpbuf, sizeof(tmpbuf)); if (!access_ok(VERIFY_WRITE, buffer, len)) return -EFAULT; - + if (*lenp < len) + len = *lenp; if (__copy_to_user(buffer, tmpbuf, len)) return -EFAULT; }
| |