| Message ID | 20240130154626.741-1-chenhx.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] nfs: fix regression in handling of fsc= option in NFSv4 | expand |
On Tue, 2024-01-30 at 23:46 +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 <chenhx.fnst@fujitsu.com> > --- > v2: > use kmemdup_nul instead of snprintf > > fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c > index d09bcfd7db89..4a23d9143d5a 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.size = len; In the other patch you were setting .size to the return value from snprintf. That does not include the NUL byte. You're including it here though. Which way is correct? > + param_fsc.string = kmemdup_nul(ctx->fscache_uniq, len, GFP_KERNEL); > + if (param_fsc.string == NULL) { > + put_fs_context(root_fc); > + return -ENOMEM; > + } > + ret = vfs_parse_fs_param(root_fc, ¶m_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;
> -----邮件原件----- > 发件人: Jeff Layton <jlayton@kernel.org> > 发送时间: 2024年1月30日 23:50 > 收件人: Chen, Hanxiao/陈 晗霄 <chenhx.fnst@fujitsu.com>; Trond Myklebust > <trond.myklebust@hammerspace.com>; Anna Schumaker <anna@kernel.org> > 抄送: linux-nfs@vger.kernel.org; Dave Wysochanski <dwysocha@redhat.com>; > David Howells <dhowells@redhat.com> > 主题: Re: [PATCH v2] nfs: fix regression in handling of fsc= option in NFSv4 > > On Tue, 2024-01-30 at 23:46 +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 <chenhx.fnst@fujitsu.com> > > --- > > v2: > > use kmemdup_nul instead of snprintf > > > > fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c > > index d09bcfd7db89..4a23d9143d5a 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.size = len; > > In the other patch you were setting .size to the return value from > snprintf. That does not include the NUL byte. You're including it here > though. Which way is correct? > It should be: len = strlen(ctx->fscache_uniq). param_fsc.size = len; for kmemdup_nul case. Will be fixed in v3. Sorry for the noise. Regards - Chen > > + param_fsc.string = kmemdup_nul(ctx->fscache_uniq, len, > GFP_KERNEL); > > + if (param_fsc.string == NULL) { > > + put_fs_context(root_fc); > > + return -ENOMEM; > > + } > > + ret = vfs_parse_fs_param(root_fc, ¶m_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; > > -- > Jeff Layton <jlayton@kernel.org>
diff --git a/fs/nfs/nfs4super.c b/fs/nfs/nfs4super.c index d09bcfd7db89..4a23d9143d5a 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.size = len; + param_fsc.string = kmemdup_nul(ctx->fscache_uniq, len, GFP_KERNEL); + if (param_fsc.string == NULL) { + put_fs_context(root_fc); + return -ENOMEM; + } + ret = vfs_parse_fs_param(root_fc, ¶m_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;
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 <chenhx.fnst@fujitsu.com> --- v2: use kmemdup_nul instead of snprintf fs/nfs/nfs4super.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)