* [PATCH] knfsd : export : Fix bug of svc_export_parse() @ 2007-03-20 9:26 Wei Yongjun 2007-03-27 1:36 ` Wei Yongjun 0 siblings, 1 reply; 4+ messages in thread From: Wei Yongjun @ 2007-03-20 9:26 UTC (permalink / raw) To: nfs, nfsv4 When I export /dev/null as root, and mount it from client, mount ntf4 command will frozen. You can test it by following command: #exportfs -i -o fsid=0,no_root_squash,insecure,rw,async *:/dev/null #mount -t nfs4 127.0.0.1:/ /tmp This is because in svc_export_parse() function, if check_export return false, after we reject the file handle, we must do cleanup the cache, which is not a format error of file handle. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- fs/nfsd/export.c.orig 2007-02-20 20:59:22.000000000 -0500 +++ fs/nfsd/export.c 2007-03-16 01:04:16.000000000 -0400 @@ -543,12 +543,13 @@ static int svc_export_parse(struct cache if (err) goto out; exp.ex_fsid = an_int; - err = check_export(nd.dentry->d_inode, exp.ex_flags); - if (err) goto out; - err = fsloc_parse(&mesg, buf, &exp.ex_fslocs); if (err) goto out; + + err = check_export(nd.dentry->d_inode, exp.ex_flags); + if (err) + set_bit(CACHE_NEGATIVE, &exp.h.flags); } expp = svc_export_lookup(&exp); ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] knfsd : export : Fix bug of svc_export_parse() 2007-03-20 9:26 [PATCH] knfsd : export : Fix bug of svc_export_parse() Wei Yongjun @ 2007-03-27 1:36 ` Wei Yongjun 2007-03-27 2:16 ` Neil Brown 0 siblings, 1 reply; 4+ messages in thread From: Wei Yongjun @ 2007-03-27 1:36 UTC (permalink / raw) To: nfsv4; +Cc: hch, nfs Nobody replied yet, but I think this is really a BUG of exportfs. exportfs command does not check so strictly, so maybe some unreasonable fh.key can be add to /proc/net/rpc/nfsd.fh/channel, but used this key to find fh.handle, this always be fail and still retry. It's this correct? > When I export /dev/null as root, and mount it from client, mount ntf4 command will frozen. > > You can test it by following command: > #exportfs -i -o fsid=0,no_root_squash,insecure,rw,async *:/dev/null > #mount -t nfs4 127.0.0.1:/ /tmp > > This is because in svc_export_parse() function, if check_export return > false, after we reject the file handle, we must do cleanup the cache, > which is not a format error of file handle. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> > > --- fs/nfsd/export.c.orig 2007-02-20 20:59:22.000000000 -0500 > +++ fs/nfsd/export.c 2007-03-16 01:04:16.000000000 -0400 > @@ -543,12 +543,13 @@ static int svc_export_parse(struct cache > if (err) goto out; > exp.ex_fsid = an_int; > > - err = check_export(nd.dentry->d_inode, exp.ex_flags); > - if (err) goto out; > - > err = fsloc_parse(&mesg, buf, &exp.ex_fslocs); > if (err) > goto out; > + > + err = check_export(nd.dentry->d_inode, exp.ex_flags); > + if (err) > + set_bit(CACHE_NEGATIVE, &exp.h.flags); > } > > expp = svc_export_lookup(&exp); > ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] knfsd : export : Fix bug of svc_export_parse() 2007-03-27 1:36 ` Wei Yongjun @ 2007-03-27 2:16 ` Neil Brown 2007-03-27 4:36 ` Wei Yongjun 0 siblings, 1 reply; 4+ messages in thread From: Neil Brown @ 2007-03-27 2:16 UTC (permalink / raw) To: Wei Yongjun; +Cc: hch, nfs, nfsv4 On Tuesday March 27, yjwei@cn.fujitsu.com wrote: > Nobody replied yet, but I think this is really a BUG of exportfs. > exportfs command does not check so strictly, so maybe some unreasonable > fh.key can be add to /proc/net/rpc/nfsd.fh/channel, but used this key to > find fh.handle, this always be fail and still retry. > > It's this correct? Sorry for not replying earlier. Yes, you have identified a real problem, but I'm not sure I agree with the first. If someone (mountd) tried to tell the kernel to export "/dev/null", it fails with an error (-ENODIR), and I think this is correct. Mountd should respond properly to this error, which it doesn't at the moment. When the request comes via the MOUNT protocol from an NFSv2 or NFSv3 client, mountd will fail the mount as it should. However when the request comes from the kernel due to an NFSv4 request, the error is just ignored. In that case we really should be telling the kernel that the filehandle is not valid by writing out an appropriate message. Maybe something like this in nfs-utils. What do you think? Signed-off-by: Neil Brown <neilb@suse.de> ### Diffstat output ./utils/mountd/cache.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff .prev/utils/mountd/cache.c ./utils/mountd/cache.c --- .prev/utils/mountd/cache.c 2007-03-27 12:11:52.000000000 +1000 +++ ./utils/mountd/cache.c 2007-03-27 12:13:40.000000000 +1000 @@ -469,7 +469,8 @@ void nfsd_fh(FILE *f) } if (found) - cache_export_ent(dom, found, found_path); + if (cache_export_ent(dom, found, found_path) < 0) + found = 0; qword_print(f, dom); qword_printint(f, fsidtype); @@ -619,8 +620,10 @@ void nfsd_export(FILE *f) } if (found) { - dump_to_cache(f, dom, path, &found->m_export); - mountlist_add(dom, path); + if (dump_to_cache(f, dom, path, &found->m_export) < 0) + dump_to_cache(f, dom, path, NULL); + else + mountlist_add(dom, path); } else { dump_to_cache(f, dom, path, NULL); } ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] knfsd : export : Fix bug of svc_export_parse() 2007-03-27 2:16 ` Neil Brown @ 2007-03-27 4:36 ` Wei Yongjun 0 siblings, 0 replies; 4+ messages in thread From: Wei Yongjun @ 2007-03-27 4:36 UTC (permalink / raw) To: Neil Brown; +Cc: nfsv4, nfs I have test some of the export entry's type: file, dir, symlink, block device, char device, socket , fifo, and dir which is a mount entry used your patch, this patch can resolve all of this problem. Thanks Neil Brown wrote: > On Tuesday March 27, yjwei@cn.fujitsu.com wrote: > >> Nobody replied yet, but I think this is really a BUG of exportfs. >> exportfs command does not check so strictly, so maybe some unreasonable >> fh.key can be add to /proc/net/rpc/nfsd.fh/channel, but used this key to >> find fh.handle, this always be fail and still retry. >> >> It's this correct? >> > > Sorry for not replying earlier. > > Yes, you have identified a real problem, but I'm not sure I agree with > the first. > > If someone (mountd) tried to tell the kernel to export "/dev/null", it > fails with an error (-ENODIR), and I think this is correct. > > Mountd should respond properly to this error, which it doesn't at the > moment. > When the request comes via the MOUNT protocol from an NFSv2 or NFSv3 > client, mountd will fail the mount as it should. > However when the request comes from the kernel due to an NFSv4 > request, the error is just ignored. In that case we really should be > telling the kernel that the filehandle is not valid by writing out an > appropriate message. > > > Maybe something like this in nfs-utils. > What do you think? > > Signed-off-by: Neil Brown <neilb@suse.de> > > ### Diffstat output > ./utils/mountd/cache.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff .prev/utils/mountd/cache.c ./utils/mountd/cache.c > --- .prev/utils/mountd/cache.c 2007-03-27 12:11:52.000000000 +1000 > +++ ./utils/mountd/cache.c 2007-03-27 12:13:40.000000000 +1000 > @@ -469,7 +469,8 @@ void nfsd_fh(FILE *f) > } > > if (found) > - cache_export_ent(dom, found, found_path); > + if (cache_export_ent(dom, found, found_path) < 0) > + found = 0; > > qword_print(f, dom); > qword_printint(f, fsidtype); > @@ -619,8 +620,10 @@ void nfsd_export(FILE *f) > } > > if (found) { > - dump_to_cache(f, dom, path, &found->m_export); > - mountlist_add(dom, path); > + if (dump_to_cache(f, dom, path, &found->m_export) < 0) > + dump_to_cache(f, dom, path, NULL); > + else > + mountlist_add(dom, path); > } else { > dump_to_cache(f, dom, path, NULL); > } > ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-03-27 4:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-03-20 9:26 [PATCH] knfsd : export : Fix bug of svc_export_parse() Wei Yongjun 2007-03-27 1:36 ` Wei Yongjun 2007-03-27 2:16 ` Neil Brown 2007-03-27 4:36 ` Wei Yongjun
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.