2008-09-01 18:51:13

by J.Bruce Fields

Subject: [PATCH] sunrpc: fix possible overrun on read of /proc/sys/sunrpc/transports

From: Cyrill Gorcunov <[email protected]>

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 <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
CC: Ingo Oeser <[email protected]>
Cc: Neil Brown <[email protected]>
Cc: Chuck Lever <[email protected]>
Cc: Greg Banks <[email protected]>
Cc: Tom Tucker <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
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 0f8c439..5231f7a 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,
void __user *buffer, size_t *lenp, loff_t *ppos)
{
char tmpbuf[256];
- int len;
+ size_t len;
+
if ((*ppos && !write) || !*lenp) {
*lenp = 0;
return 0;
}
- if (write)
- return -EINVAL;
- else {
- len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
- if (!access_ok(VERIFY_WRITE, buffer, len))
- return -EFAULT;
-
- if (__copy_to_user(buffer, tmpbuf, len))
- return -EFAULT;
- }
- *lenp -= len;
- *ppos += len;
- return 0;
+ len = svc_print_xprts(tmpbuf, sizeof(tmpbuf));
+ return simple_read_from_buffer(buffer, *lenp, ppos, tmpbuf, len);
}

static int
--
1.5.5.rc1

2008-09-01 18:51:07

by J.Bruce Fields

Subject: [PATCH] nfsd: fix buffer overrun decoding NFSv4 acl

The array we kmalloc() here is not large enough.

Thanks to Johann Dahm and David Richter for bug report and testing.

Signed-off-by: J. Bruce Fields <[email protected]>
Cc: David Richter <[email protected]>
Tested-by: Johann Dahm <[email protected]>
---
fs/nfsd/nfs4acl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/nfsd/nfs4acl.c b/fs/nfsd/nfs4acl.c
index b6ed383..54b8b41 100644
--- a/fs/nfsd/nfs4acl.c
+++ b/fs/nfsd/nfs4acl.c
@@ -443,7 +443,7 @@ init_state(struct posix_acl_state *state, int cnt)
* enough space for either:
*/
alloc = sizeof(struct posix_ace_state_array)
- + cnt*sizeof(struct posix_ace_state);
+ + cnt*sizeof(struct posix_user_ace_state);
state->users = kzalloc(alloc, GFP_KERNEL);
if (!state->users)
return -ENOMEM;
--
1.5.5.rc1