diff options
author | Cyrill Gorcunov <gorcunov@gmail.com> | 2008-08-31 11:25:49 -0400 |
---|---|---|
committer | J. Bruce Fields <bfields@citi.umich.edu> | 2008-09-01 14:24:24 -0400 |
commit | 27df6f25ff218072e0e879a96beeb398a79cdbc8 (patch) | |
tree | 92156018e74a963b2abb94bd51a7e7d6b6a72f34 | |
parent | c228c24bf1138d4757dbe20615df655815446da3 (diff) |
sunrpc: fix possible overrun on read of /proc/sys/sunrpc/transports
Vegard Nossum reported
----------------------
> I noticed that something weird is going on with /proc/sys/sunrpc/transports.
> This file is generated in net/sunrpc/sysctl.c, function proc_do_xprt(). When
> I "cat" this file, I get the expected output:
> $ cat /proc/sys/sunrpc/transports
> tcp 1048576
> udp 32768
> But I think that it does not check the length of the buffer supplied by
> userspace to read(). With my original program, I found that the stack was
> being overwritten by the characters above, even when the length given to
> read() was just 1.
David Wagner added (among other things) that copy_to_user could be
probably used here.
Ingo Oeser suggested to use simple_read_from_buffer() here.
The conclusion is that proc_do_xprt doesn't check for userside buffer
size indeed so fix this by using Ingo's suggestion.
Reported-by: Vegard Nossum <vegard.nossum@gmail.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
CC: Ingo Oeser <ioe-lkml@rameria.de>
Cc: Neil Brown <neilb@suse.de>
Cc: Chuck Lever <chuck.lever@oracle.com>
Cc: Greg Banks <gnb@sgi.com>
Cc: Tom Tucker <tom@opengridcomputing.com>
Signed-off-by: J. Bruce Fields <bfields@citi.umich.edu>
-rw-r--r-- | net/sunrpc/sysctl.c | 18 |
1 files changed, 4 insertions, 14 deletions
diff --git a/net/sunrpc/sysctl.c b/net/sunrpc/sysctl.c index 0f8c439b848a..5231f7aaac0e 100644 --- a/net/sunrpc/sysctl.c +++ b/net/sunrpc/sysctl.c | |||
@@ -60,24 +60,14 @@ static int proc_do_xprt(ctl_table *table, int write, struct file *file, | |||
60 | void __user *buffer, size_t *lenp, loff_t *ppos) | 60 | void __user *buffer, size_t *lenp, loff_t *ppos) |
61 | { | 61 | { |
62 | char tmpbuf[256]; | 62 | char tmpbuf[256]; |
63 | int len; | 63 | size_t len; |
64 | |||
64 | if ((*ppos && !write) || !*lenp) { | 65 | if ((*ppos && !write) || !*lenp) { |
65 | *lenp = 0; | 66 | *lenp = 0; |
66 | return 0; | 67 | return 0; |
67 | } | 68 | } |
68 | if (write) | 69 | len = svc_print_xprts(tmpbuf, sizeof(tmpbuf)); |
69 | return -EINVAL; | 70 | return simple_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len); |
70 | else { | ||
71 | len = svc_print_xprts(tmpbuf, sizeof(tmpbuf)); | ||
72 | if (!access_ok(VERIFY_WRITE, buffer, len)) | ||
73 | return -EFAULT; | ||
74 | |||
75 | if (__copy_to_user(buffer, tmpbuf, len)) | ||
76 | return -EFAULT; | ||
77 | } | ||
78 | *lenp -= len; | ||
79 | *ppos += len; | ||
80 | return 0; | ||
81 | } | 71 | } |
82 | 72 | ||
83 | static int | 73 | static int |