* [RFC: 2.6 patch] include/linux/nfsd/nfsfh.h: fix a NULL
@ 2006-11-12 13:17 ` Adrian Bunk
0 siblings, 0 replies; 7+ messages in thread
From: Adrian Bunk @ 2006-11-12 13:17 UTC (permalink / raw)
To: neilb; +Cc: nfs, linux-kernel
dereference
Reply-To:
Fcc: =sent-mail
When we know fhp->fh_dentry is NULL, a code path where it's being
dereferenced isn't a good choice.
Spotted by the coverity checker.
Signed-off-by: Adrian Bunk <bunk@stusta.de>
--- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 14:13:34.000000000 +0100
+++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 14:13:49.000000000 +0100
@@ -330,8 +330,7 @@ fh_unlock(struct svc_fh *fhp)
{
if (!fhp->fh_dentry)
printk(KERN_ERR "fh_unlock: fh not verified!\n");
-
- if (fhp->fh_locked) {
+ else if (fhp->fh_locked) {
fill_post_wcc(fhp);
mutex_unlock(&fhp->fh_dentry->d_inode->i_mutex);
fhp->fh_locked = 0;
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
NFS maillist - NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs
^ permalink raw reply [flat|nested] 7+ messages in thread* [RFC: 2.6 patch] include/linux/nfsd/nfsfh.h: fix a NULL @ 2006-11-12 13:17 ` Adrian Bunk 0 siblings, 0 replies; 7+ messages in thread From: Adrian Bunk @ 2006-11-12 13:17 UTC (permalink / raw) To: neilb; +Cc: nfs, linux-kernel dereference Reply-To: Fcc: =sent-mail When we know fhp->fh_dentry is NULL, a code path where it's being dereferenced isn't a good choice. Spotted by the coverity checker. Signed-off-by: Adrian Bunk <bunk@stusta.de> --- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 14:13:34.000000000 +0100 +++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 14:13:49.000000000 +0100 @@ -330,8 +330,7 @@ fh_unlock(struct svc_fh *fhp) { if (!fhp->fh_dentry) printk(KERN_ERR "fh_unlock: fh not verified!\n"); - - if (fhp->fh_locked) { + else if (fhp->fh_locked) { fill_post_wcc(fhp); mutex_unlock(&fhp->fh_dentry->d_inode->i_mutex); fhp->fh_locked = 0; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC: 2.6 patch] include/linux/nfsd/nfsfh.h: fix a NULL 2006-11-12 13:17 ` Adrian Bunk @ 2006-11-12 15:58 ` Trond Myklebust -1 siblings, 0 replies; 7+ messages in thread From: Trond Myklebust @ 2006-11-12 15:58 UTC (permalink / raw) To: Adrian Bunk; +Cc: nfs, neilb, linux-kernel On Sun, 2006-11-12 at 14:17 +0100, Adrian Bunk wrote: > dereference > Reply-To: > Fcc: =sent-mail > > When we know fhp->fh_dentry is NULL, a code path where it's being > dereferenced isn't a good choice. > > Spotted by the coverity checker. > > Signed-off-by: Adrian Bunk <bunk@stusta.de> > > --- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 14:13:34.000000000 +0100 > +++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 14:13:49.000000000 +0100 > @@ -330,8 +330,7 @@ fh_unlock(struct svc_fh *fhp) > { > if (!fhp->fh_dentry) > printk(KERN_ERR "fh_unlock: fh not verified!\n"); > - > - if (fhp->fh_locked) { > + else if (fhp->fh_locked) { > fill_post_wcc(fhp); > mutex_unlock(&fhp->fh_dentry->d_inode->i_mutex); > fhp->fh_locked = 0; > This issue has come up on lkml before. Please just convert that check for fhp->fh_dentry into a BUG_ON(). Trond ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [NFS] [RFC: 2.6 patch] include/linux/nfsd/nfsfh.h: fix a NULL @ 2006-11-12 15:58 ` Trond Myklebust 0 siblings, 0 replies; 7+ messages in thread From: Trond Myklebust @ 2006-11-12 15:58 UTC (permalink / raw) To: Adrian Bunk; +Cc: neilb, nfs, linux-kernel On Sun, 2006-11-12 at 14:17 +0100, Adrian Bunk wrote: > dereference > Reply-To: > Fcc: =sent-mail > > When we know fhp->fh_dentry is NULL, a code path where it's being > dereferenced isn't a good choice. > > Spotted by the coverity checker. > > Signed-off-by: Adrian Bunk <bunk@stusta.de> > > --- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 14:13:34.000000000 +0100 > +++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 14:13:49.000000000 +0100 > @@ -330,8 +330,7 @@ fh_unlock(struct svc_fh *fhp) > { > if (!fhp->fh_dentry) > printk(KERN_ERR "fh_unlock: fh not verified!\n"); > - > - if (fhp->fh_locked) { > + else if (fhp->fh_locked) { > fill_post_wcc(fhp); > mutex_unlock(&fhp->fh_dentry->d_inode->i_mutex); > fhp->fh_locked = 0; > This issue has come up on lkml before. Please just convert that check for fhp->fh_dentry into a BUG_ON(). Trond ^ permalink raw reply [flat|nested] 7+ messages in thread
* [2.6 patch] include/linux/nfsd/nfsfh.h:fh_unlock(): change an error to a BUG_ON() 2006-11-12 15:58 ` [NFS] " Trond Myklebust (?) @ 2006-11-12 17:48 ` Adrian Bunk 2006-11-13 4:52 ` Neil Brown -1 siblings, 1 reply; 7+ messages in thread From: Adrian Bunk @ 2006-11-12 17:48 UTC (permalink / raw) To: Trond Myklebust; +Cc: neilb, nfs, linux-kernel On Sun, Nov 12, 2006 at 10:58:47AM -0500, Trond Myklebust wrote: > On Sun, 2006-11-12 at 14:17 +0100, Adrian Bunk wrote: > > dereference > > Reply-To: > > Fcc: =sent-mail > > > > When we know fhp->fh_dentry is NULL, a code path where it's being > > dereferenced isn't a good choice. > > > > Spotted by the coverity checker. > > > > Signed-off-by: Adrian Bunk <bunk@stusta.de> > > > > --- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 14:13:34.000000000 +0100 > > +++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 14:13:49.000000000 +0100 > > @@ -330,8 +330,7 @@ fh_unlock(struct svc_fh *fhp) > > { > > if (!fhp->fh_dentry) > > printk(KERN_ERR "fh_unlock: fh not verified!\n"); > > - > > - if (fhp->fh_locked) { > > + else if (fhp->fh_locked) { > > fill_post_wcc(fhp); > > mutex_unlock(&fhp->fh_dentry->d_inode->i_mutex); > > fhp->fh_locked = 0; > > > > This issue has come up on lkml before. Please just convert that check > for fhp->fh_dentry into a BUG_ON(). Patch below. > Trond cu Adrian <-- snip --> !fhp->fh_dentry is a condition that mustn't ever happen (and we might run into additional trouble later). Signed-off-by: Adrian Bunk <bunk@stusta.de> --- linux-2.6/include/linux/nfsd/nfsfh.h.old 2006-11-12 18:37:43.000000000 +0100 +++ linux-2.6/include/linux/nfsd/nfsfh.h 2006-11-12 18:38:03.000000000 +0100 @@ -328,8 +328,7 @@ static inline void fh_unlock(struct svc_fh *fhp) { - if (!fhp->fh_dentry) - printk(KERN_ERR "fh_unlock: fh not verified!\n"); + BUG_ON(!fhp->fh_dentry); if (fhp->fh_locked) { fill_post_wcc(fhp); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [2.6 patch] include/linux/nfsd/nfsfh.h:fh_unlock(): change an error to a BUG_ON() 2006-11-12 17:48 ` [2.6 patch] include/linux/nfsd/nfsfh.h:fh_unlock(): change an error to a BUG_ON() Adrian Bunk @ 2006-11-13 4:52 ` Neil Brown 0 siblings, 0 replies; 7+ messages in thread From: Neil Brown @ 2006-11-13 4:52 UTC (permalink / raw) To: Adrian Bunk; +Cc: nfs, linux-kernel, Trond Myklebust On Sunday November 12, bunk@stusta.de wrote: > > > > This issue has come up on lkml before. Please just convert that check > > for fhp->fh_dentry into a BUG_ON(). > > Patch below. > Thanks. I might that the opportunity to do a bit more cleaning up here and suggest this version . NeilBrown --------------------------- Subject: Replace some warning ins nfsfh.h with BUG_ON or WARN_ON A couple of the warning will be followed by an Oops if they ever fire, so may as well be BUG_ON. Another isn't obviously fatal but has never been known to fire, so make it a WARN_ON. Cc: Adrian Bunk <bunk@stusta.de> Signed-off-by: Neil Brown <neilb@suse.de> ### Diffstat output ./include/linux/nfsd/nfsfh.h | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff .prev/include/linux/nfsd/nfsfh.h ./include/linux/nfsd/nfsfh.h --- .prev/include/linux/nfsd/nfsfh.h 2006-11-13 15:44:24.000000000 +1100 +++ ./include/linux/nfsd/nfsfh.h 2006-11-13 15:47:02.000000000 +1100 @@ -217,11 +217,7 @@ void fh_put(struct svc_fh *); static __inline__ struct svc_fh * fh_copy(struct svc_fh *dst, struct svc_fh *src) { - if (src->fh_dentry || src->fh_locked) { - struct dentry *dentry = src->fh_dentry; - printk(KERN_ERR "fh_copy: copying %s/%s, already verified!\n", - dentry->d_parent->d_name.name, dentry->d_name.name); - } + WARN_ON(src->fh_dentry || src->fh_locked); *dst = *src; return dst; @@ -300,10 +296,8 @@ fh_lock_nested(struct svc_fh *fhp, unsig dfprintk(FILEOP, "nfsd: fh_lock(%s) locked = %d\n", SVCFH_fmt(fhp), fhp->fh_locked); - if (!fhp->fh_dentry) { - printk(KERN_ERR "fh_lock: fh not verified!\n"); - return; - } + BUG_ON(!dentry); + if (fhp->fh_locked) { printk(KERN_WARNING "fh_lock: %s/%s already locked!\n", dentry->d_parent->d_name.name, dentry->d_name.name); @@ -328,8 +322,7 @@ fh_lock(struct svc_fh *fhp) static inline void fh_unlock(struct svc_fh *fhp) { - if (!fhp->fh_dentry) - printk(KERN_ERR "fh_unlock: fh not verified!\n"); + BUG_ON(!fhp->fh_dentry); if (fhp->fh_locked) { fill_post_wcc(fhp); ------------------------------------------------------------------------- Using Tomcat but need to do more? Need to support web services, security? Get stuff done quickly with pre-integrated technology to make your job easier Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642 _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [2.6 patch] include/linux/nfsd/nfsfh.h:fh_unlock(): change an error to a BUG_ON() @ 2006-11-13 4:52 ` Neil Brown 0 siblings, 0 replies; 7+ messages in thread From: Neil Brown @ 2006-11-13 4:52 UTC (permalink / raw) To: Adrian Bunk; +Cc: Trond Myklebust, nfs, linux-kernel On Sunday November 12, bunk@stusta.de wrote: > > > > This issue has come up on lkml before. Please just convert that check > > for fhp->fh_dentry into a BUG_ON(). > > Patch below. > Thanks. I might that the opportunity to do a bit more cleaning up here and suggest this version . NeilBrown --------------------------- Subject: Replace some warning ins nfsfh.h with BUG_ON or WARN_ON A couple of the warning will be followed by an Oops if they ever fire, so may as well be BUG_ON. Another isn't obviously fatal but has never been known to fire, so make it a WARN_ON. Cc: Adrian Bunk <bunk@stusta.de> Signed-off-by: Neil Brown <neilb@suse.de> ### Diffstat output ./include/linux/nfsd/nfsfh.h | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff .prev/include/linux/nfsd/nfsfh.h ./include/linux/nfsd/nfsfh.h --- .prev/include/linux/nfsd/nfsfh.h 2006-11-13 15:44:24.000000000 +1100 +++ ./include/linux/nfsd/nfsfh.h 2006-11-13 15:47:02.000000000 +1100 @@ -217,11 +217,7 @@ void fh_put(struct svc_fh *); static __inline__ struct svc_fh * fh_copy(struct svc_fh *dst, struct svc_fh *src) { - if (src->fh_dentry || src->fh_locked) { - struct dentry *dentry = src->fh_dentry; - printk(KERN_ERR "fh_copy: copying %s/%s, already verified!\n", - dentry->d_parent->d_name.name, dentry->d_name.name); - } + WARN_ON(src->fh_dentry || src->fh_locked); *dst = *src; return dst; @@ -300,10 +296,8 @@ fh_lock_nested(struct svc_fh *fhp, unsig dfprintk(FILEOP, "nfsd: fh_lock(%s) locked = %d\n", SVCFH_fmt(fhp), fhp->fh_locked); - if (!fhp->fh_dentry) { - printk(KERN_ERR "fh_lock: fh not verified!\n"); - return; - } + BUG_ON(!dentry); + if (fhp->fh_locked) { printk(KERN_WARNING "fh_lock: %s/%s already locked!\n", dentry->d_parent->d_name.name, dentry->d_name.name); @@ -328,8 +322,7 @@ fh_lock(struct svc_fh *fhp) static inline void fh_unlock(struct svc_fh *fhp) { - if (!fhp->fh_dentry) - printk(KERN_ERR "fh_unlock: fh not verified!\n"); + BUG_ON(!fhp->fh_dentry); if (fhp->fh_locked) { fill_post_wcc(fhp); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-11-13 4:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-11-12 13:17 [RFC: 2.6 patch] include/linux/nfsd/nfsfh.h: fix a NULL Adrian Bunk 2006-11-12 13:17 ` Adrian Bunk 2006-11-12 15:58 ` Trond Myklebust 2006-11-12 15:58 ` [NFS] " Trond Myklebust 2006-11-12 17:48 ` [2.6 patch] include/linux/nfsd/nfsfh.h:fh_unlock(): change an error to a BUG_ON() Adrian Bunk 2006-11-13 4:52 ` Neil Brown 2006-11-13 4:52 ` Neil Brown
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.