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

[Jeff Layton <jlayton@xxxxxxxxxx>]

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@xxxxxxxxxxx>
> ---
> 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, &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;

-- 
Jeff Layton <jlayton@xxxxxxxxxx>