* [PATCH] nfsd: Fix oops when parsing a 0 length export @ 2011-11-18 10:14 Sasha Levin 2011-11-22 20:28 ` J. Bruce Fields 2012-01-16 11:52 ` [patch] nfsd: oopses in cache_parse() Dan Carpenter 0 siblings, 2 replies; 11+ messages in thread From: Sasha Levin @ 2011-11-18 10:14 UTC (permalink / raw) To: linux-kernel; +Cc: Sasha Levin, J. Bruce Fields, Neil Brown, linux-nfs expkey_parse() oopses when handling a 0 length export. This is easily triggerable from usermode by writing 0 bytes into '/proc/[proc id]/net/rpc/nfsd.fh/channel'. Below is the log: [ 1402.286893] BUG: unable to handle kernel paging request at ffff880077c49fff [ 1402.287632] IP: [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 [ 1402.287632] PGD 2206063 PUD 1fdfd067 PMD 1ffbc067 PTE 8000000077c49160 [ 1402.287632] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC [ 1402.287632] CPU 1 [ 1402.287632] Pid: 20198, comm: trinity Not tainted 3.2.0-rc2-sasha-00058-gc65cd37 #6 [ 1402.287632] RIP: 0010:[<ffffffff812b4b99>] [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 [ 1402.287632] RSP: 0018:ffff880077f0fd68 EFLAGS: 00010292 [ 1402.287632] RAX: ffff880077c49fff RBX: 00000000ffffffea RCX: 0000000001043400 [ 1402.287632] RDX: 0000000000000000 RSI: ffff880077c4a000 RDI: ffffffff82283de0 [ 1402.287632] RBP: ffff880077f0fe18 R08: 0000000000000001 R09: ffff880000000000 [ 1402.287632] R10: 0000000000000000 R11: 0000000000000001 R12: ffff880077c4a000 [ 1402.287632] R13: ffffffff82283de0 R14: 0000000001043400 R15: ffffffff82283de0 [ 1402.287632] FS: 00007f25fec3f700(0000) GS:ffff88007d400000(0000) knlGS:0000000000000000 [ 1402.287632] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b [ 1402.287632] CR2: ffff880077c49fff CR3: 0000000077e1d000 CR4: 00000000000406e0 [ 1402.287632] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1402.287632] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 1402.287632] Process trinity (pid: 20198, threadinfo ffff880077f0e000, task ffff880077db17b0) [ 1402.287632] Stack: [ 1402.287632] ffff880077db17b0 ffff880077c4a000 ffff880077f0fdb8 ffffffff810b411e [ 1402.287632] ffff880000000000 ffff880077db17b0 ffff880077c4a000 ffffffff82283de0 [ 1402.287632] 0000000001043400 ffffffff82283de0 ffff880077f0fde8 ffffffff81111f63 [ 1402.287632] Call Trace: [ 1402.287632] [<ffffffff810b411e>] ? lock_release+0x1af/0x1bc [ 1402.287632] [<ffffffff81111f63>] ? might_fault+0x97/0x9e [ 1402.287632] [<ffffffff81111f1a>] ? might_fault+0x4e/0x9e [ 1402.287632] [<ffffffff81a8bcf2>] cache_do_downcall+0x3e/0x4f [ 1402.287632] [<ffffffff81a8c950>] cache_write.clone.16+0xbb/0x130 [ 1402.287632] [<ffffffff81a8c9df>] ? cache_write_pipefs+0x1a/0x1a [ 1402.287632] [<ffffffff81a8c9f8>] cache_write_procfs+0x19/0x1b [ 1402.287632] [<ffffffff8118dc54>] proc_reg_write+0x8e/0xad [ 1402.287632] [<ffffffff8113fe81>] vfs_write+0xaa/0xfd [ 1402.287632] [<ffffffff8114142d>] ? fget_light+0x35/0x9e [ 1402.287632] [<ffffffff8113ff8b>] sys_write+0x48/0x6f [ 1402.287632] [<ffffffff81bbdb92>] system_call_fastpath+0x16/0x1b [ 1402.287632] Code: c0 c9 c3 55 48 63 d2 48 89 e5 48 8d 44 32 ff 41 57 41 56 41 55 41 54 53 bb ea ff ff ff 48 81 ec 88 00 00 00 48 89 b5 58 ff ff ff [ 1402.287632] 38 0a 0f 85 89 02 00 00 c6 00 00 48 8b 3d 44 4a e5 01 48 85 [ 1402.287632] RIP [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 [ 1402.287632] RSP <ffff880077f0fd68> [ 1402.287632] CR2: ffff880077c49fff [ 1402.287632] ---[ end trace 368ef53ff773a5e3 ]--- Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Neil Brown <neilb@suse.de> Cc: linux-nfs@vger.kernel.org Signed-off-by: Sasha Levin <levinsasha928@gmail.com> --- fs/nfsd/export.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index 62f3b90..5f312ab 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -87,7 +87,7 @@ static int expkey_parse(struct cache_detail *cd, char *mesg, int mlen) struct svc_expkey key; struct svc_expkey *ek = NULL; - if (mesg[mlen-1] != '\n') + if (mlen < 1 || mesg[mlen-1] != '\n') return -EINVAL; mesg[mlen-1] = 0; -- 1.7.8.rc1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] nfsd: Fix oops when parsing a 0 length export 2011-11-18 10:14 [PATCH] nfsd: Fix oops when parsing a 0 length export Sasha Levin @ 2011-11-22 20:28 ` J. Bruce Fields 2011-11-22 20:33 ` Sasha Levin 2012-01-16 11:52 ` [patch] nfsd: oopses in cache_parse() Dan Carpenter 1 sibling, 1 reply; 11+ messages in thread From: J. Bruce Fields @ 2011-11-22 20:28 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-kernel, Neil Brown, linux-nfs On Fri, Nov 18, 2011 at 12:14:49PM +0200, Sasha Levin wrote: > expkey_parse() oopses when handling a 0 length export. This is easily > triggerable from usermode by writing 0 bytes into > '/proc/[proc id]/net/rpc/nfsd.fh/channel'. Thanks--how did you run across this? --b. > > Below is the log: > > [ 1402.286893] BUG: unable to handle kernel paging request at ffff880077c49fff > [ 1402.287632] IP: [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 > [ 1402.287632] PGD 2206063 PUD 1fdfd067 PMD 1ffbc067 PTE 8000000077c49160 > [ 1402.287632] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC > [ 1402.287632] CPU 1 > [ 1402.287632] Pid: 20198, comm: trinity Not tainted 3.2.0-rc2-sasha-00058-gc65cd37 #6 > [ 1402.287632] RIP: 0010:[<ffffffff812b4b99>] [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 > [ 1402.287632] RSP: 0018:ffff880077f0fd68 EFLAGS: 00010292 > [ 1402.287632] RAX: ffff880077c49fff RBX: 00000000ffffffea RCX: 0000000001043400 > [ 1402.287632] RDX: 0000000000000000 RSI: ffff880077c4a000 RDI: ffffffff82283de0 > [ 1402.287632] RBP: ffff880077f0fe18 R08: 0000000000000001 R09: ffff880000000000 > [ 1402.287632] R10: 0000000000000000 R11: 0000000000000001 R12: ffff880077c4a000 > [ 1402.287632] R13: ffffffff82283de0 R14: 0000000001043400 R15: ffffffff82283de0 > [ 1402.287632] FS: 00007f25fec3f700(0000) GS:ffff88007d400000(0000) knlGS:0000000000000000 > [ 1402.287632] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b > [ 1402.287632] CR2: ffff880077c49fff CR3: 0000000077e1d000 CR4: 00000000000406e0 > [ 1402.287632] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 1402.287632] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 > [ 1402.287632] Process trinity (pid: 20198, threadinfo ffff880077f0e000, task ffff880077db17b0) > [ 1402.287632] Stack: > [ 1402.287632] ffff880077db17b0 ffff880077c4a000 ffff880077f0fdb8 ffffffff810b411e > [ 1402.287632] ffff880000000000 ffff880077db17b0 ffff880077c4a000 ffffffff82283de0 > [ 1402.287632] 0000000001043400 ffffffff82283de0 ffff880077f0fde8 ffffffff81111f63 > [ 1402.287632] Call Trace: > [ 1402.287632] [<ffffffff810b411e>] ? lock_release+0x1af/0x1bc > [ 1402.287632] [<ffffffff81111f63>] ? might_fault+0x97/0x9e > [ 1402.287632] [<ffffffff81111f1a>] ? might_fault+0x4e/0x9e > [ 1402.287632] [<ffffffff81a8bcf2>] cache_do_downcall+0x3e/0x4f > [ 1402.287632] [<ffffffff81a8c950>] cache_write.clone.16+0xbb/0x130 > [ 1402.287632] [<ffffffff81a8c9df>] ? cache_write_pipefs+0x1a/0x1a > [ 1402.287632] [<ffffffff81a8c9f8>] cache_write_procfs+0x19/0x1b > [ 1402.287632] [<ffffffff8118dc54>] proc_reg_write+0x8e/0xad > [ 1402.287632] [<ffffffff8113fe81>] vfs_write+0xaa/0xfd > [ 1402.287632] [<ffffffff8114142d>] ? fget_light+0x35/0x9e > [ 1402.287632] [<ffffffff8113ff8b>] sys_write+0x48/0x6f > [ 1402.287632] [<ffffffff81bbdb92>] system_call_fastpath+0x16/0x1b > [ 1402.287632] Code: c0 c9 c3 55 48 63 d2 48 89 e5 48 8d 44 32 ff 41 57 41 56 41 55 41 54 53 bb ea ff ff ff 48 81 ec 88 00 00 00 48 89 b5 58 ff ff ff > [ 1402.287632] 38 0a 0f 85 89 02 00 00 c6 00 00 48 8b 3d 44 4a e5 01 48 85 > [ 1402.287632] RIP [<ffffffff812b4b99>] expkey_parse+0x28/0x2e1 > [ 1402.287632] RSP <ffff880077f0fd68> > [ 1402.287632] CR2: ffff880077c49fff > [ 1402.287632] ---[ end trace 368ef53ff773a5e3 ]--- > > Cc: "J. Bruce Fields" <bfields@fieldses.org> > Cc: Neil Brown <neilb@suse.de> > Cc: linux-nfs@vger.kernel.org > Signed-off-by: Sasha Levin <levinsasha928@gmail.com> > --- > fs/nfsd/export.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index 62f3b90..5f312ab 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -87,7 +87,7 @@ static int expkey_parse(struct cache_detail *cd, char *mesg, int mlen) > struct svc_expkey key; > struct svc_expkey *ek = NULL; > > - if (mesg[mlen-1] != '\n') > + if (mlen < 1 || mesg[mlen-1] != '\n') > return -EINVAL; > mesg[mlen-1] = 0; > > -- > 1.7.8.rc1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] nfsd: Fix oops when parsing a 0 length export 2011-11-22 20:28 ` J. Bruce Fields @ 2011-11-22 20:33 ` Sasha Levin 2011-11-22 20:59 ` J. Bruce Fields 0 siblings, 1 reply; 11+ messages in thread From: Sasha Levin @ 2011-11-22 20:33 UTC (permalink / raw) To: J. Bruce Fields; +Cc: linux-kernel, Neil Brown, linux-nfs On Tue, 2011-11-22 at 15:28 -0500, J. Bruce Fields wrote: > On Fri, Nov 18, 2011 at 12:14:49PM +0200, Sasha Levin wrote: > > expkey_parse() oopses when handling a 0 length export. This is easily > > triggerable from usermode by writing 0 bytes into > > '/proc/[proc id]/net/rpc/nfsd.fh/channel'. > > Thanks--how did you run across this? > > --b. Tried running a fuzzer on the system. -- Sasha. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] nfsd: Fix oops when parsing a 0 length export 2011-11-22 20:33 ` Sasha Levin @ 2011-11-22 20:59 ` J. Bruce Fields 0 siblings, 0 replies; 11+ messages in thread From: J. Bruce Fields @ 2011-11-22 20:59 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-kernel, Neil Brown, linux-nfs On Tue, Nov 22, 2011 at 10:33:29PM +0200, Sasha Levin wrote: > On Tue, 2011-11-22 at 15:28 -0500, J. Bruce Fields wrote: > > On Fri, Nov 18, 2011 at 12:14:49PM +0200, Sasha Levin wrote: > > > expkey_parse() oopses when handling a 0 length export. This is easily > > > triggerable from usermode by writing 0 bytes into > > > '/proc/[proc id]/net/rpc/nfsd.fh/channel'. > > > > Thanks--how did you run across this? > > > > --b. > > Tried running a fuzzer on the system. OK, thanks. As far as I can tell this could only be hit by a privileged user doing something stupid. And the bug has been there forever. So I'm inclined just to queue this up for 3.3. --b. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch] nfsd: oopses in cache_parse() 2011-11-18 10:14 [PATCH] nfsd: Fix oops when parsing a 0 length export Sasha Levin 2011-11-22 20:28 ` J. Bruce Fields @ 2012-01-16 11:52 ` Dan Carpenter 2012-01-16 15:50 ` J. Bruce Fields 1 sibling, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2012-01-16 11:52 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-kernel, J. Bruce Fields, Neil Brown, linux-nfs [-- Attachment #1: Type: text/plain, Size: 1867 bytes --] We fixed expkey_parse() in b2ea70afad "nfsd: Fix oops when parsing a 0 length export" but there are other cache_parse() implimentations which have the same issue. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- Since half of the implimentations get this wrong, maybe we should just check for this in cache_do_downcall(). Is there ever a valid reason to pass a zero length string to cache_parse()? diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index cf8a6bd..1d147a8 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -496,7 +496,7 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen) struct svc_export exp = {}, *expp; int an_int; - if (mesg[mlen-1] != '\n') + if (mlen < 1 || mesg[mlen - 1] != '\n') return -EINVAL; mesg[mlen-1] = 0; diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c index a6e711a..d945d71 100644 --- a/fs/nfs/dns_resolve.c +++ b/fs/nfs/dns_resolve.c @@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) ssize_t len; int ret = -EINVAL; - if (buf[buflen-1] != '\n') + if (buflen < 1 || buf[buflen - 1] != '\n') goto out; buf[buflen-1] = '\0'; diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c index 9409627..f8456a4 100644 --- a/fs/nfsd/nfs4idmap.c +++ b/fs/nfsd/nfs4idmap.c @@ -202,7 +202,7 @@ idtoname_parse(struct cache_detail *cd, char *buf, int buflen) int len; int error = -EINVAL; - if (buf[buflen - 1] != '\n') + if (buflen < 1 || buf[buflen - 1] != '\n') return (-EINVAL); buf[buflen - 1]= '\0'; @@ -378,7 +378,7 @@ nametoid_parse(struct cache_detail *cd, char *buf, int buflen) char *buf1; int error = -EINVAL; - if (buf[buflen - 1] != '\n') + if (buflen < 1 || buf[buflen - 1] != '\n') return (-EINVAL); buf[buflen - 1]= '\0'; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch] nfsd: oopses in cache_parse() 2012-01-16 11:52 ` [patch] nfsd: oopses in cache_parse() Dan Carpenter @ 2012-01-16 15:50 ` J. Bruce Fields 2012-01-18 9:55 ` Dan Carpenter 2012-01-18 9:56 ` [patch] nfsd: don't allow zero length strings " Dan Carpenter 0 siblings, 2 replies; 11+ messages in thread From: J. Bruce Fields @ 2012-01-16 15:50 UTC (permalink / raw) To: Dan Carpenter; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs On Mon, Jan 16, 2012 at 02:52:58PM +0300, Dan Carpenter wrote: > We fixed expkey_parse() in b2ea70afad "nfsd: Fix oops when parsing a > 0 length export" but there are other cache_parse() implimentations > which have the same issue. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > Since half of the implimentations get this wrong, maybe we should > just check for this in cache_do_downcall(). Is there ever a valid > reason to pass a zero length string to cache_parse()? I don't think so, no. Checking in cache_do_downcall() sounds like a good idea. --b. > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index cf8a6bd..1d147a8 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -496,7 +496,7 @@ static int svc_export_parse(struct cache_detail *cd, char *mesg, int mlen) > struct svc_export exp = {}, *expp; > int an_int; > > - if (mesg[mlen-1] != '\n') > + if (mlen < 1 || mesg[mlen - 1] != '\n') > return -EINVAL; > mesg[mlen-1] = 0; > > diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c > index a6e711a..d945d71 100644 > --- a/fs/nfs/dns_resolve.c > +++ b/fs/nfs/dns_resolve.c > @@ -217,7 +217,7 @@ static int nfs_dns_parse(struct cache_detail *cd, char *buf, int buflen) > ssize_t len; > int ret = -EINVAL; > > - if (buf[buflen-1] != '\n') > + if (buflen < 1 || buf[buflen - 1] != '\n') > goto out; > buf[buflen-1] = '\0'; > > diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c > index 9409627..f8456a4 100644 > --- a/fs/nfsd/nfs4idmap.c > +++ b/fs/nfsd/nfs4idmap.c > @@ -202,7 +202,7 @@ idtoname_parse(struct cache_detail *cd, char *buf, int buflen) > int len; > int error = -EINVAL; > > - if (buf[buflen - 1] != '\n') > + if (buflen < 1 || buf[buflen - 1] != '\n') > return (-EINVAL); > buf[buflen - 1]= '\0'; > > @@ -378,7 +378,7 @@ nametoid_parse(struct cache_detail *cd, char *buf, int buflen) > char *buf1; > int error = -EINVAL; > > - if (buf[buflen - 1] != '\n') > + if (buflen < 1 || buf[buflen - 1] != '\n') > return (-EINVAL); > buf[buflen - 1]= '\0'; > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] nfsd: oopses in cache_parse() 2012-01-16 15:50 ` J. Bruce Fields @ 2012-01-18 9:55 ` Dan Carpenter 2012-01-18 9:56 ` [patch] nfsd: don't allow zero length strings " Dan Carpenter 1 sibling, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2012-01-18 9:55 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs [-- Attachment #1: Type: text/plain, Size: 853 bytes --] On Mon, Jan 16, 2012 at 10:50:31AM -0500, J. Bruce Fields wrote: > On Mon, Jan 16, 2012 at 02:52:58PM +0300, Dan Carpenter wrote: > > We fixed expkey_parse() in b2ea70afad "nfsd: Fix oops when parsing a > > 0 length export" but there are other cache_parse() implimentations > > which have the same issue. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > Since half of the implimentations get this wrong, maybe we should > > just check for this in cache_do_downcall(). Is there ever a valid > > reason to pass a zero length string to cache_parse()? > > I don't think so, no. Checking in cache_do_downcall() sounds like a > good idea. Yeah. I checked and none of the cache_parse() implementations do anything with a zero length string. Ignore that patch, I'll send another. regards, dan carpenter [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch] nfsd: don't allow zero length strings in cache_parse() 2012-01-16 15:50 ` J. Bruce Fields 2012-01-18 9:55 ` Dan Carpenter @ 2012-01-18 9:56 ` Dan Carpenter 2012-01-18 22:41 ` J. Bruce Fields 1 sibling, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2012-01-18 9:56 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs [-- Attachment #1: Type: text/plain, Size: 577 bytes --] There is no point in passing a zero length string here and quite a few of that cache_parse() implementations will Oops if count is zero. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 465df9a..8c6598e 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -828,6 +828,8 @@ static ssize_t cache_do_downcall(char *kaddr, const char __user *buf, { ssize_t ret; + if (count == 0) + return -EINVAL; if (copy_from_user(kaddr, buf, count)) return -EFAULT; kaddr[count] = '\0'; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch] nfsd: don't allow zero length strings in cache_parse() 2012-01-18 9:56 ` [patch] nfsd: don't allow zero length strings " Dan Carpenter @ 2012-01-18 22:41 ` J. Bruce Fields 2012-01-20 7:48 ` [patch] nfsd: remove some unneeded checks Dan Carpenter 0 siblings, 1 reply; 11+ messages in thread From: J. Bruce Fields @ 2012-01-18 22:41 UTC (permalink / raw) To: Dan Carpenter; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs On Wed, Jan 18, 2012 at 12:56:02PM +0300, Dan Carpenter wrote: > There is no point in passing a zero length string here and quite a > few of that cache_parse() implementations will Oops if count is > zero. Thanks, I'll apply. Could we clean up any checks in cache_parse() that are unnecessary now? --b. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c > index 465df9a..8c6598e 100644 > --- a/net/sunrpc/cache.c > +++ b/net/sunrpc/cache.c > @@ -828,6 +828,8 @@ static ssize_t cache_do_downcall(char *kaddr, const char __user *buf, > { > ssize_t ret; > > + if (count == 0) > + return -EINVAL; > if (copy_from_user(kaddr, buf, count)) > return -EFAULT; > kaddr[count] = '\0'; ^ permalink raw reply [flat|nested] 11+ messages in thread
* [patch] nfsd: remove some unneeded checks 2012-01-18 22:41 ` J. Bruce Fields @ 2012-01-20 7:48 ` Dan Carpenter 2012-01-20 18:35 ` J. Bruce Fields 0 siblings, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2012-01-20 7:48 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs [-- Attachment #1: Type: text/plain, Size: 940 bytes --] We check for zero length strings in the caller now, so these aren't needed. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c index cf8a6bd..8e9689a 100644 --- a/fs/nfsd/export.c +++ b/fs/nfsd/export.c @@ -87,7 +87,7 @@ static int expkey_parse(struct cache_detail *cd, char *mesg, int mlen) struct svc_expkey key; struct svc_expkey *ek = NULL; - if (mlen < 1 || mesg[mlen-1] != '\n') + if (mesg[mlen - 1] != '\n') return -EINVAL; mesg[mlen-1] = 0; diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c index 01153ea..6ab3573 100644 --- a/net/sunrpc/svcauth_unix.c +++ b/net/sunrpc/svcauth_unix.c @@ -509,7 +509,7 @@ static int unix_gid_parse(struct cache_detail *cd, time_t expiry; struct unix_gid ug, *ugp; - if (mlen <= 0 || mesg[mlen-1] != '\n') + if (mesg[mlen - 1] != '\n') return -EINVAL; mesg[mlen-1] = 0; [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [patch] nfsd: remove some unneeded checks 2012-01-20 7:48 ` [patch] nfsd: remove some unneeded checks Dan Carpenter @ 2012-01-20 18:35 ` J. Bruce Fields 0 siblings, 0 replies; 11+ messages in thread From: J. Bruce Fields @ 2012-01-20 18:35 UTC (permalink / raw) To: Dan Carpenter; +Cc: Sasha Levin, linux-kernel, Neil Brown, linux-nfs On Fri, Jan 20, 2012 at 10:48:18AM +0300, Dan Carpenter wrote: > We check for zero length strings in the caller now, so these aren't > needed. Thanks! Applying.--b. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c > index cf8a6bd..8e9689a 100644 > --- a/fs/nfsd/export.c > +++ b/fs/nfsd/export.c > @@ -87,7 +87,7 @@ static int expkey_parse(struct cache_detail *cd, char *mesg, int mlen) > struct svc_expkey key; > struct svc_expkey *ek = NULL; > > - if (mlen < 1 || mesg[mlen-1] != '\n') > + if (mesg[mlen - 1] != '\n') > return -EINVAL; > mesg[mlen-1] = 0; > > diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c > index 01153ea..6ab3573 100644 > --- a/net/sunrpc/svcauth_unix.c > +++ b/net/sunrpc/svcauth_unix.c > @@ -509,7 +509,7 @@ static int unix_gid_parse(struct cache_detail *cd, > time_t expiry; > struct unix_gid ug, *ugp; > > - if (mlen <= 0 || mesg[mlen-1] != '\n') > + if (mesg[mlen - 1] != '\n') > return -EINVAL; > mesg[mlen-1] = 0; > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-01-20 18:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-18 10:14 [PATCH] nfsd: Fix oops when parsing a 0 length export Sasha Levin 2011-11-22 20:28 ` J. Bruce Fields 2011-11-22 20:33 ` Sasha Levin 2011-11-22 20:59 ` J. Bruce Fields 2012-01-16 11:52 ` [patch] nfsd: oopses in cache_parse() Dan Carpenter 2012-01-16 15:50 ` J. Bruce Fields 2012-01-18 9:55 ` Dan Carpenter 2012-01-18 9:56 ` [patch] nfsd: don't allow zero length strings " Dan Carpenter 2012-01-18 22:41 ` J. Bruce Fields 2012-01-20 7:48 ` [patch] nfsd: remove some unneeded checks Dan Carpenter 2012-01-20 18:35 ` J. Bruce Fields
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox