On Thu, 2025-06-19 at 07:31 +1000, NeilBrown wrote:
> write_foo functions are called to handle IO to files in /proc/fs/nfsd/.
> The 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.
>
> Reported-by: Li Lingfeng <lilingfeng3@xxxxxxxxxx>
> Signed-off-by: NeilBrown <neil@xxxxxxxxxx>
> ---
> fs/nfsd/nfsctl.c | 115 +++++++++++++++++++++++++++++++----------------
> 1 file changed, 77 insertions(+), 38 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 3f3e9f6c4250..3710a1992d17 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -200,27 +200,18 @@ static inline struct net *netns(struct file *file)
> return file_inode(file)->i_sb->s_fs_info;
> }
>
> -/*
> - * write_unlock_ip - Release all locks used by a client
> - *
> - * Experimental.
> - *
> - * Input:
> - * buf: '\n'-terminated C string containing a
> - * presentation format IP address
> - * size: length of C string in @buf
> - * Output:
> - * On success: returns zero if all specified locks were released;
> - * returns one if one or more locks were not released
> - * On error: return code is negative errno value
> - */
> -static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
> +static ssize_t __write_unlock_ip(struct file *file, char *buf, size_t size)
> {
> struct sockaddr_storage address;
> struct sockaddr *sap = (struct sockaddr *)&address;
> size_t salen = sizeof(address);
> char *fo_path;
> struct net *net = netns(file);
> + struct nfsd_net *nn = net_generic(net, nfsd_net_id);
> +
> + if (!nn->nfsd_serv)
> + /* There cannot be any files to unlock */
> + return -EINVAL;
>
> /* sanity check */
> if (size == 0)
> @@ -241,24 +232,39 @@ static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
> }
>
> /*
> - * write_unlock_fs - Release all locks on a local file system
> + * write_unlock_ip - Release all locks used by a client
> *
> * Experimental.
> *
> * Input:
> - * buf: '\n'-terminated C string containing the
> - * absolute pathname of a local file system
> + * buf: '\n'-terminated C string containing a
> + * presentation format IP address
> * size: length of C string in @buf
> * Output:
> * On success: returns zero if all specified locks were released;
> * returns one if one or more locks were not released
> * On error: return code is negative errno value
> */
> -static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
> +static ssize_t write_unlock_ip(struct file *file, char *buf, size_t size)
> +{
> + ssize_t rv;
> +
> + mutex_lock(&nfsd_mutex);
> + rv = __write_unlock_ip(file, buf, size);
> + mutex_unlock(&nfsd_mutex);
> + return rv;
> +}
> +
> +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);
> +
> + if (!nn->nfsd_serv)
> + /* There cannot be any files to unlock */
> + return -EINVAL;
>
> /* sanity check */
> if (size == 0)
> @@ -291,6 +297,30 @@ static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
> return error;
> }
>
> +/*
> + * write_unlock_fs - Release all locks on a local file system
> + *
> + * Experimental.
> + *
> + * Input:
> + * buf: '\n'-terminated C string containing the
> + * absolute pathname of a local file system
> + * size: length of C string in @buf
> + * Output:
> + * On success: returns zero if all specified locks were released;
> + * returns one if one or more locks were not released
> + * On error: return code is negative errno value
> + */
> +static ssize_t write_unlock_fs(struct file *file, char *buf, size_t size)
> +{
> + ssize_t rv;
> +
> + mutex_lock(&nfsd_mutex);
> + rv = __write_unlock_fs(file, buf, size);
> + mutex_unlock(&nfsd_mutex);
> + return rv;
> +}
> +
> /*
> * write_filehandle - Get a variable-length NFS file handle by path
> *
> @@ -1053,6 +1083,29 @@ static ssize_t write_recoverydir(struct file *file, char *buf, size_t size)
> }
> #endif
>
> +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);
> +
> + if (size > 0) {
> + switch(buf[0]) {
> + case 'Y':
> + case 'y':
> + case '1':
> + if (!nn->nfsd_serv)
> + return -EBUSY;
> + trace_nfsd_end_grace(netns(file));
> + nfsd4_end_grace(nn);
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%c\n",
> + nn->grace_ended ? 'Y' : 'N');
> +}
> +
> /*
> * write_v4_end_grace - release grace period for nfsd's v4.x lock manager
> *
> @@ -1075,27 +1128,13 @@ static ssize_t write_recoverydir(struct file *file, char *buf, size_t size)
> */
> 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);
> -
> - if (size > 0) {
> - switch(buf[0]) {
> - case 'Y':
> - case 'y':
> - case '1':
> - if (!nn->nfsd_serv)
> - return -EBUSY;
> - trace_nfsd_end_grace(netns(file));
> - nfsd4_end_grace(nn);
> - break;
> - default:
> - return -EINVAL;
> - }
> - }
> + ssize_t rv;
>
> - return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%c\n",
> - nn->grace_ended ? 'Y' : 'N');
> + mutex_lock(&nfsd_mutex);
> + rv = __write_v4_end_grace(file, buf, size);
> + mutex_unlock(&nfsd_mutex);
> + return rv;
> }
> -
> #endif
>
> /*----------------------------------------------------------------------------*/
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
