Re: [PATCH 1/3] nfsd: provide proper locking for all write_ function

[Li Lingfeng <lilingfeng3@huawei.com>]

Thank you for the patch, it did fix the issue.

在 2025/6/21 7:33, NeilBrown 写道:
> write_foo functions are called to handle IO to files in /proc/fs/nfsd/.
> They can be called at any time and so generally need locking to ensure
> they don't happen at an awkward time.
>
> Many already take nfsd_mutex and check if nfsd_serv has been set.  This
> ensures they only run when the server is fully configured.
>
> write_filehandle() does *not* need locking.  It interacts with the
> export table which is set up when the netns is set up, so it is always
> valid and it has its own locking.  write_filehandle() is needed before
> the nfs server is started so checking nfsd_serv would be wrong.
>
> The remaining files which do not have any locking are
> write_v4_end_grace(), write_unlock_ip(), and write_unlock_fs().
> None of these make sense when the nfs server is not running and there is
> evidence that write_v4_end_grace() can race with ->client_tracking_op
> setup/shutdown and cause problems.
>
> This patch adds locking to these three and ensures the "unlock"
> functions abort if ->nfsd_serv is not set.  It uses
>      guard(mutex)(&nfsd_mutex);
> so there is no need to ensure we unlock on every patch.
>
> Reported-by: Li Lingfeng <lilingfeng3@huawei.com>
> Signed-off-by: NeilBrown <neil@brown.name>
> ---
>   fs/nfsd/nfsctl.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 3f3e9f6c4250..0e7e89dc730b 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -221,6 +221,12 @@ static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
>   	size_t salen = sizeof(address);
>   	char *fo_path;
>   	struct net *net = netns(file);
> +	struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +
> +	guard(mutex)(&nfsd_mutex);
> +	if (!nn->nfsd_serv)
> +		/* There cannot be any files to unlock */
> +		return -EINVAL;
>   
>   	/* sanity check */
>   	if (size == 0)
> @@ -259,6 +265,12 @@ static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
>   	struct path path;
>   	char *fo_path;
>   	int error;
> +	struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
> +
> +	guard(mutex)(&nfsd_mutex);
> +	if (!nn->nfsd_serv)
> +		/* There cannot be any files to unlock */
> +		return -EINVAL;
>   
>   	/* sanity check */
>   	if (size == 0)
> @@ -1053,6 +1065,7 @@ static ssize_t write_recoverydir(struct file *file, char *buf, size_t size)
>   }
>   #endif
>   
> +
>   /*
>    * write_v4_end_grace - release grace period for nfsd's v4.x lock manager
>    *
> @@ -1077,6 +1090,7 @@ static ssize_t write_v4_end_grace(struct file *file, char *buf, size_t size)
>   {
>   	struct nfsd_net *nn = net_generic(netns(file), nfsd_net_id);
>   
> +	guard(mutex)(&nfsd_mutex);
>   	if (size > 0) {
>   		switch(buf[0]) {
>   		case 'Y':
Tested-by: Li Lingfeng <lilingfeng3@huawei.com>