| Message ID | 20190212182734.GA4509@kroah.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rpc: properly check debugfs dentry before using it | expand |
On Tue, 2019-02-12 at 19:27 +0100, Greg Kroah-Hartman wrote: > debugfs can now report an error code if something went wrong instead of > just NULL. So if the return value is to be used as a "real" dentry, it > needs to be checked if it is an error before dereferencing it. > > This is now happening because of ff9fb72bc077 ("debugfs: return error > values, not NULL"), but why debugfs files are not being created properly > is an older issue, probably one that has always been there and should > probably be looked at... > > Cc: "J. Bruce Fields" <bfields@fieldses.org> > Cc: Jeff Layton <jlayton@kernel.org> > Cc: Trond Myklebust <trond.myklebust@hammerspace.com> > Cc: Anna Schumaker <anna.schumaker@netapp.com> > Cc: linux-nfs@vger.kernel.org > Cc: netdev@vger.kernel.org > Reported-by: David Howells <dhowells@redhat.com> > Tested-by: David Howells <dhowells@redhat.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > net/sunrpc/debugfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > I can take this through my tree if people don't object, or it can go > through the NFS tree. It does need to get merged before 5.0-final > though. I'm planning another bugfixes pull for 5.0, so I can take this patch and send it with the others this week. Thanks! Anna > > I also have a "larger" debugfs cleanup patch for this file, but that's > not really 5.0-final material and I will send it out later. > > thanks, > > greg k-h > > diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c > index 45a033329cd4..19bb356230ed 100644 > --- a/net/sunrpc/debugfs.c > +++ b/net/sunrpc/debugfs.c > @@ -146,7 +146,7 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt) > rcu_read_lock(); > xprt = rcu_dereference(clnt->cl_xprt); > /* no "debugfs" dentry? Don't bother with the symlink. */ > - if (!xprt->debugfs) { > + if (IS_ERR_OR_NULL(xprt->debugfs)) { > rcu_read_unlock(); > return; > }
On Tue, Feb 12, 2019 at 08:52:53PM +0000, Schumaker, Anna wrote: > On Tue, 2019-02-12 at 19:27 +0100, Greg Kroah-Hartman wrote: > > debugfs can now report an error code if something went wrong instead of > > just NULL. So if the return value is to be used as a "real" dentry, it > > needs to be checked if it is an error before dereferencing it. > > > > This is now happening because of ff9fb72bc077 ("debugfs: return error > > values, not NULL"), but why debugfs files are not being created properly > > is an older issue, probably one that has always been there and should > > probably be looked at... > > > > Cc: "J. Bruce Fields" <bfields@fieldses.org> > > Cc: Jeff Layton <jlayton@kernel.org> > > Cc: Trond Myklebust <trond.myklebust@hammerspace.com> > > Cc: Anna Schumaker <anna.schumaker@netapp.com> > > Cc: linux-nfs@vger.kernel.org > > Cc: netdev@vger.kernel.org > > Reported-by: David Howells <dhowells@redhat.com> > > Tested-by: David Howells <dhowells@redhat.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > --- > > net/sunrpc/debugfs.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > I can take this through my tree if people don't object, or it can go > > through the NFS tree. It does need to get merged before 5.0-final > > though. > > I'm planning another bugfixes pull for 5.0, so I can take this patch and send it > with the others this week. Wonderful, thanks for doing this, and sorry for the regression. greg k-h
diff --git a/net/sunrpc/debugfs.c b/net/sunrpc/debugfs.c index 45a033329cd4..19bb356230ed 100644 --- a/net/sunrpc/debugfs.c +++ b/net/sunrpc/debugfs.c @@ -146,7 +146,7 @@ rpc_clnt_debugfs_register(struct rpc_clnt *clnt) rcu_read_lock(); xprt = rcu_dereference(clnt->cl_xprt); /* no "debugfs" dentry? Don't bother with the symlink. */ - if (!xprt->debugfs) { + if (IS_ERR_OR_NULL(xprt->debugfs)) { rcu_read_unlock(); return; }
debugfs can now report an error code if something went wrong instead of just NULL. So if the return value is to be used as a "real" dentry, it needs to be checked if it is an error before dereferencing it. This is now happening because of ff9fb72bc077 ("debugfs: return error values, not NULL"), but why debugfs files are not being created properly is an older issue, probably one that has always been there and should probably be looked at... Cc: "J. Bruce Fields" <bfields@fieldses.org> Cc: Jeff Layton <jlayton@kernel.org> Cc: Trond Myklebust <trond.myklebust@hammerspace.com> Cc: Anna Schumaker <anna.schumaker@netapp.com> Cc: linux-nfs@vger.kernel.org Cc: netdev@vger.kernel.org Reported-by: David Howells <dhowells@redhat.com> Tested-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- net/sunrpc/debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) I can take this through my tree if people don't object, or it can go through the NFS tree. It does need to get merged before 5.0-final though. I also have a "larger" debugfs cleanup patch for this file, but that's not really 5.0-final material and I will send it out later. thanks, greg k-h