2024-03-05 03:44:21

by Chen Hanxiao

Subject: [PATCH] NFS: add a tracepoint for uniquifier of fscache

Add a tracepoint to the mount fsc=xxx option

Signed-off-by: Chen Hanxiao <[email protected]>
---
fs/nfs/fs_context.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index 853e8d609bb3..fd8813222cd2 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -652,6 +652,9 @@ static int nfs_fs_context_parse_param(struct fs_context *fc,
ctx->fscache_uniq = NULL;
break;
case Opt_fscache:
+ if (!param->string)
+ goto out_invalid_value;
+ trace_nfs_mount_assign(param->key, param->string);
ctx->options |= NFS_OPTION_FSCACHE;
kfree(ctx->fscache_uniq);
ctx->fscache_uniq = param->string;
--
2.39.1

2024-03-05 15:29:00

by Trond Myklebust

Subject: Re: [PATCH] NFS: add a tracepoint for uniquifier of fscache

On Tue, 2024-03-05 at 11:41 +0800, Chen Hanxiao wrote:
> [You don't often get email from [email protected]. Learn why
> this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Add a tracepoint to the mount fsc=xxx option
>
> Signed-off-by: Chen Hanxiao <[email protected]>
> ---
>  fs/nfs/fs_context.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> index 853e8d609bb3..fd8813222cd2 100644
> --- a/fs/nfs/fs_context.c
> +++ b/fs/nfs/fs_context.c
> @@ -652,6 +652,9 @@ static int nfs_fs_context_parse_param(struct
> fs_context *fc,
>                 ctx->fscache_uniq = NULL;
>                 break;
>         case Opt_fscache:
> +               if (!param->string)
> +                       goto out_invalid_value;
> +               trace_nfs_mount_assign(param->key, param->string);

The description does not match the contents of the patch. Why would we
need that extra check on top of the ones made by fs_param_is_string()?

>                 ctx->options |= NFS_OPTION_FSCACHE;
>                 kfree(ctx->fscache_uniq);
>                 ctx->fscache_uniq = param->string;
> --
> 2.39.1
>

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]

2024-03-06 04:09:14

by Chen Hanxiao

Subject: Re: [PATCH] NFS: add a tracepoint for uniquifier of fscache

> -----邮件原件-----
> 发件人: Trond Myklebust <[email protected]>
> 发送时间: 2024年3月5日 23:28
> 收件人: [email protected];
> 抄送: [email protected]; [email protected]
> 主题: Re: [PATCH] NFS: add a tracepoint for uniquifier of fscache
>
> On Tue, 2024-03-05 at 11:41 +0800, Chen Hanxiao wrote:
> > [You don't often get email from [email protected]. Learn why
> > this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Add a tracepoint to the mount fsc=xxx option
> >
> > Signed-off-by: Chen Hanxiao <[email protected]>
> > ---
> >  fs/nfs/fs_context.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
> > index 853e8d609bb3..fd8813222cd2 100644
> > --- a/fs/nfs/fs_context.c
> > +++ b/fs/nfs/fs_context.c
> > @@ -652,6 +652,9 @@ static int nfs_fs_context_parse_param(struct
> > fs_context *fc,
> >                 ctx->fscache_uniq = NULL;
> >                 break;
> >         case Opt_fscache:
> > +               if (!param->string)
> > +                       goto out_invalid_value;
> > +               trace_nfs_mount_assign(param->key, param->string);
>
> The description does not match the contents of the patch. Why would we
> need that extra check on top of the ones made by fs_param_is_string()?
>

I take 5559405df652 ("nfs: fix possible null-ptr-deref when parsing param") for reference.
Maybe I misunderstand that fix.
I'll post a v2 without the null check, and with a proper description about commit message.

Regards,
- Chen