* [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup()
@ 2024-08-26 7:06 Yan Zhen
2024-08-26 9:39 ` Jiri Pirko
2024-08-26 11:52 ` Jeff Layton
0 siblings, 2 replies; 3+ messages in thread
From: Yan Zhen @ 2024-08-26 7:06 UTC (permalink / raw)
To: chuck.lever, jlayton, trondmy, anna, davem, edumazet, kuba,
pabeni
Cc: neilb, okorniev, Dai.Ngo, tom, linux-nfs, netdev, linux-kernel,
opensource.kernel, Yan Zhen
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);
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup()
2024-08-26 7:06 [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup() Yan Zhen
@ 2024-08-26 9:39 ` Jiri Pirko
2024-08-26 11:52 ` Jeff Layton
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Pirko @ 2024-08-26 9:39 UTC (permalink / raw)
To: Yan Zhen
Cc: chuck.lever, jlayton, trondmy, anna, davem, edumazet, kuba,
pabeni, neilb, okorniev, Dai.Ngo, tom, linux-nfs, netdev,
linux-kernel, opensource.kernel
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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup()
2024-08-26 7:06 [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup() Yan Zhen
2024-08-26 9:39 ` Jiri Pirko
@ 2024-08-26 11:52 ` Jeff Layton
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2024-08-26 11:52 UTC (permalink / raw)
To: Yan Zhen, chuck.lever, trondmy, anna, davem, edumazet, kuba,
pabeni
Cc: neilb, okorniev, Dai.Ngo, tom, linux-nfs, netdev, linux-kernel,
opensource.kernel
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.
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-26 11:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-26 7:06 [PATCH v1] net: sunrpc: Fix error checking for d_hash_and_lookup() Yan Zhen
2024-08-26 9:39 ` Jiri Pirko
2024-08-26 11:52 ` Jeff Layton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).