2024-01-24 14:52:02

by Chen Hanxiao

Subject: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4

Setting the uniquifier for fscache via the fsc= mount
option is currently broken in NFSv4.

Fix this by passing fscache_uniq to root_fc if possible.

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

diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
index d09bcfd7db89..669d7f65ef9c 100644
--- a/fs/nfs/nfs4super.c
+++ b/fs/nfs/nfs4super.c
@@ -145,6 +145,7 @@ static int do_nfs4_mount(struct nfs_server *server,
const char *export_path)
{
struct nfs_fs_context *root_ctx;
+ struct nfs_fs_context *ctx;
struct fs_context *root_fc;
struct vfsmount *root_mnt;
struct dentry *dentry;
@@ -157,6 +158,12 @@ static int do_nfs4_mount(struct nfs_server *server,
.dirfd = -1,
};

+ struct fs_parameter param_fsc = {
+ .key = "fsc",
+ .type = fs_value_is_string,
+ .dirfd = -1,
+ };
+
if (IS_ERR(server))
return PTR_ERR(server);

@@ -168,9 +175,26 @@ static int do_nfs4_mount(struct nfs_server *server,
kfree(root_fc->source);
root_fc->source = NULL;

+ ctx = nfs_fc2context(fc);
root_ctx = nfs_fc2context(root_fc);
root_ctx->internal = true;
root_ctx->server = server;
+
+ if (ctx->fscache_uniq) {
+ len = strlen(ctx->fscache_uniq) + 1;
+ param_fsc.string = kmalloc(len, GFP_KERNEL);
+ if (param_fsc.string == NULL) {
+ put_fs_context(root_fc);
+ return -ENOMEM;
+ }
+ param_fsc.size = snprintf(param_fsc.string, len, "%s", ctx->fscache_uniq);
+ ret = vfs_parse_fs_param(root_fc, &param_fsc);
+ kfree(param_fsc.string);
+ if (ret < 0) {
+ put_fs_context(root_fc);
+ return ret;
+ }
+ }
/* We leave export_path unset as it's not used to find the root. */

len = strlen(hostname) + 5;
--
2.39.1

2024-01-30 12:19:10

by Chen Hanxiao

Subject: Re: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4

> -----?ʼ?ԭ??-----
> ??????: Chen Hanxiao <[email protected]>
> ????ʱ??: 2024??1??24?? 22:38
> ?ռ???: Trond Myklebust <[email protected]>; Anna
> Schumaker <[email protected]>
> ????: Jeff Layton <[email protected]>; [email protected]
> ????: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4
>
> Setting the uniquifier for fscache via the fsc= mount
> option is currently broken in NFSv4.
>
> Fix this by passing fscache_uniq to root_fc if possible.
>
> Signed-off-by: Chen Hanxiao <[email protected]>
> ---
> fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>


Ping?

Regards,
- Chen

2024-01-30 14:32:54

by Jeff Layton

Subject: Re: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4

On Wed, 2024-01-24 at 22:37 +0800, Chen Hanxiao wrote:
> Setting the uniquifier for fscache via the fsc= mount
> option is currently broken in NFSv4.
>
> Fix this by passing fscache_uniq to root_fc if possible.
>
> Signed-off-by: Chen Hanxiao <[email protected]>
> ---
> fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
> index d09bcfd7db89..669d7f65ef9c 100644
> --- a/fs/nfs/nfs4super.c
> +++ b/fs/nfs/nfs4super.c
> @@ -145,6 +145,7 @@ static int do_nfs4_mount(struct nfs_server *server,
> const char *export_path)
> {
> struct nfs_fs_context *root_ctx;
> + struct nfs_fs_context *ctx;
> struct fs_context *root_fc;
> struct vfsmount *root_mnt;
> struct dentry *dentry;
> @@ -157,6 +158,12 @@ static int do_nfs4_mount(struct nfs_server *server,
> .dirfd = -1,
> };
>
> + struct fs_parameter param_fsc = {
> + .key = "fsc",
> + .type = fs_value_is_string,
> + .dirfd = -1,
> + };
> +
> if (IS_ERR(server))
> return PTR_ERR(server);
>
> @@ -168,9 +175,26 @@ static int do_nfs4_mount(struct nfs_server *server,
> kfree(root_fc->source);
> root_fc->source = NULL;
>
> + ctx = nfs_fc2context(fc);
> root_ctx = nfs_fc2context(root_fc);
> root_ctx->internal = true;
> root_ctx->server = server;
> +
> + if (ctx->fscache_uniq) {
> + len = strlen(ctx->fscache_uniq) + 1;
> + param_fsc.string = kmalloc(len, GFP_KERNEL);
> + if (param_fsc.string == NULL) {
> + put_fs_context(root_fc);
> + return -ENOMEM;
> + }
> + param_fsc.size = snprintf(param_fsc.string, len, "%s", ctx->fscache_uniq);

After getting the size, you can just call kmemdup_nul instead of doing
this manually.

> + ret = vfs_parse_fs_param(root_fc, &param_fsc);
> + kfree(param_fsc.string);
> + if (ret < 0) {
> + put_fs_context(root_fc);
> + return ret;
> + }
> + }
> /* We leave export_path unset as it's not used to find the root. */
>
> len = strlen(hostname) + 5;

The problem and fix looks about right though.

--
Jeff Layton <[email protected]>

2024-01-30 15:35:50

by Chen Hanxiao

Subject: Re: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4

> -----?ʼ?ԭ??-----
> ??????: Jeff Layton <[email protected]>
> ????ʱ??: 2024??1??30?? 22:24
> ?ռ???: Chen, Hanxiao/?? ???? <[email protected]>; Trond
> Myklebust <[email protected]>; Anna Schumaker
> <[email protected]>
> ????: [email protected]; Dave Wysochanski
> <[email protected]>; David Howells <[email protected]>
> ????: Re: [PATCH] nfs: fix regression in handling of fsc= option in NFSv4
>
> On Wed, 2024-01-24 at 22:37 +0800, Chen Hanxiao wrote:
> > Setting the uniquifier for fscache via the fsc= mount
> > option is currently broken in NFSv4.
> >
> > Fix this by passing fscache_uniq to root_fc if possible.
> >
> > Signed-off-by: Chen Hanxiao <[email protected]>
> > ---
> > fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> >
> > diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c
> > index d09bcfd7db89..669d7f65ef9c 100644
> > --- a/fs/nfs/nfs4super.c
> > +++ b/fs/nfs/nfs4super.c
> > @@ -145,6 +145,7 @@ static int do_nfs4_mount(struct nfs_server *server,
> > const char *export_path)
> > {
> > struct nfs_fs_context *root_ctx;
> > + struct nfs_fs_context *ctx;
> > struct fs_context *root_fc;
> > struct vfsmount *root_mnt;
> > struct dentry *dentry;
> > @@ -157,6 +158,12 @@ static int do_nfs4_mount(struct nfs_server
> *server,
> > .dirfd = -1,
> > };
> >
> > + struct fs_parameter param_fsc = {
> > + .key = "fsc",
> > + .type = fs_value_is_string,
> > + .dirfd = -1,
> > + };
> > +
> > if (IS_ERR(server))
> > return PTR_ERR(server);
> >
> > @@ -168,9 +175,26 @@ static int do_nfs4_mount(struct nfs_server
> *server,
> > kfree(root_fc->source);
> > root_fc->source = NULL;
> >
> > + ctx = nfs_fc2context(fc);
> > root_ctx = nfs_fc2context(root_fc);
> > root_ctx->internal = true;
> > root_ctx->server = server;
> > +
> > + if (ctx->fscache_uniq) {
> > + len = strlen(ctx->fscache_uniq) + 1;
> > + param_fsc.string = kmalloc(len, GFP_KERNEL);
> > + if (param_fsc.string == NULL) {
> > + put_fs_context(root_fc);
> > + return -ENOMEM;
> > + }
> > + param_fsc.size = snprintf(param_fsc.string, len, "%s",
> ctx->fscache_uniq);
>
> After getting the size, you can just call kmemdup_nul instead of doing
> this manually.
>
> > + ret = vfs_parse_fs_param(root_fc, &param_fsc);
> > + kfree(param_fsc.string);
> > + if (ret < 0) {
> > + put_fs_context(root_fc);
> > + return ret;
> > + }
> > + }
> > /* We leave export_path unset as it's not used to find the root. */
> >
> > len = strlen(hostname) + 5;
>
> The problem and fix looks about right though.
>

Thanks, will send v2 soon.

Regards,
- Chen
> --
> Jeff Layton <[email protected]>