| Message ID | 20240826070659.2287801-1-yanzhen@vivo.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1] net: sunrpc: Fix error checking for d_hash_and_lookup() | expand |
Mon, Aug 26, 2024 at 09:06:59AM CEST, yanzhen@vivo.com wrote: >The d_hash_and_lookup() function returns either an error pointer or NULL. > >It might be more appropriate to check error using IS_ERR_OR_NULL(). > >Signed-off-by: Yan Zhen <yanzhen@vivo.com> You need to provide a "fixes" tag blaming the commit that introduced the bug. >--- > net/sunrpc/rpc_pipe.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c >index 910a5d850d04..fd03dd46b1f2 100644 >--- a/net/sunrpc/rpc_pipe.c >+++ b/net/sunrpc/rpc_pipe.c >@@ -1306,7 +1306,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) > > /* We should never get this far if "gssd" doesn't exist */ > gssd_dentry = d_hash_and_lookup(root, &q); >- if (!gssd_dentry) >+ if (IS_ERR_OR_NULL(gssd_dentry)) > return ERR_PTR(-ENOENT); > > ret = rpc_populate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1, NULL); >@@ -1318,7 +1318,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) > q.name = gssd_dummy_clnt_dir[0].name; > q.len = strlen(gssd_dummy_clnt_dir[0].name); > clnt_dentry = d_hash_and_lookup(gssd_dentry, &q); >- if (!clnt_dentry) { >+ if (IS_ERR_OR_NULL(clnt_dentry)) { > __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1); > pipe_dentry = ERR_PTR(-ENOENT); > goto out; >-- >2.34.1 > >
On Mon, 2024-08-26 at 15:06 +0800, Yan Zhen wrote: > The d_hash_and_lookup() function returns either an error pointer or NULL. > > It might be more appropriate to check error using IS_ERR_OR_NULL(). > > Signed-off-by: Yan Zhen <yanzhen@vivo.com> > --- > net/sunrpc/rpc_pipe.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c > index 910a5d850d04..fd03dd46b1f2 100644 > --- a/net/sunrpc/rpc_pipe.c > +++ b/net/sunrpc/rpc_pipe.c > @@ -1306,7 +1306,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) > > /* We should never get this far if "gssd" doesn't exist */ > gssd_dentry = d_hash_and_lookup(root, &q); > - if (!gssd_dentry) > + if (IS_ERR_OR_NULL(gssd_dentry)) > return ERR_PTR(-ENOENT); This is the routine that fills out rpc_pipefs directory. If this returns an IS_ERR value, then we should probably return PTR_ERR(gssd_dentry) instead of -ENOENT. > > ret = rpc_populate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1, NULL); > @@ -1318,7 +1318,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) > q.name = gssd_dummy_clnt_dir[0].name; > q.len = strlen(gssd_dummy_clnt_dir[0].name); > clnt_dentry = d_hash_and_lookup(gssd_dentry, &q); > - if (!clnt_dentry) { > + if (IS_ERR_OR_NULL(clnt_dentry)) { > __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1); > pipe_dentry = ERR_PTR(-ENOENT); > goto out; Ditto here. Nice catch, either way though. This definitely should be fixed.
diff --git a/net/sunrpc/rpc_pipe.c b/net/sunrpc/rpc_pipe.c index 910a5d850d04..fd03dd46b1f2 100644 --- a/net/sunrpc/rpc_pipe.c +++ b/net/sunrpc/rpc_pipe.c @@ -1306,7 +1306,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) /* We should never get this far if "gssd" doesn't exist */ gssd_dentry = d_hash_and_lookup(root, &q); - if (!gssd_dentry) + if (IS_ERR_OR_NULL(gssd_dentry)) return ERR_PTR(-ENOENT); ret = rpc_populate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1, NULL); @@ -1318,7 +1318,7 @@ rpc_gssd_dummy_populate(struct dentry *root, struct rpc_pipe *pipe_data) q.name = gssd_dummy_clnt_dir[0].name; q.len = strlen(gssd_dummy_clnt_dir[0].name); clnt_dentry = d_hash_and_lookup(gssd_dentry, &q); - if (!clnt_dentry) { + if (IS_ERR_OR_NULL(clnt_dentry)) { __rpc_depopulate(gssd_dentry, gssd_dummy_clnt_dir, 0, 1); pipe_dentry = ERR_PTR(-ENOENT); goto out;
The d_hash_and_lookup() function returns either an error pointer or NULL. It might be more appropriate to check error using IS_ERR_OR_NULL(). Signed-off-by: Yan Zhen <yanzhen@vivo.com> --- net/sunrpc/rpc_pipe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)