Re: buffer overflow in /proc/sys/sunrpc/transports

[Cyrill Gorcunov <gorcunov@gmail.com>]

[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;
 	}