linux-kernel - Re: [PATCH v2] NFSD: Disallow layoutget during grace period

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <2be32654-242b-4c4c-b909-9058767b1b53@oracle.com>
Date: Fri, 5 Sep 2025 09:41:18 -0400
From: Chuck Lever <chuck.lever@...cle.com>
To: Sergey Bashirov <sergeybashirov@...il.com>,
        Jeff Layton <jlayton@...nel.org>, NeilBrown <neil@...wn.name>,
        Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>,
        Tom Talpey <tom@...pey.com>
Cc: linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Konstantin Evtushenko <koevtushenko@...dex.com>
Subject: Re: [PATCH v2] NFSD: Disallow layoutget during grace period

On 9/4/25 11:54 AM, Chuck Lever wrote:
> On 9/3/25 3:34 PM, Sergey Bashirov wrote:
>> When the block/scsi layout server is recovering from a reboot and is in a
>> grace period, any operation that may result in deletion or reallocation of
>> block extents should not be allowed. See RFC 8881, section 18.43.3.
>>
>> If multiple clients write data to the same file, rebooting the server
>> during writing can result in the file corruption. Observed this behavior
>> while testing pNFS block volume setup.
>>
>> Co-developed-by: Konstantin Evtushenko <koevtushenko@...dex.com>
>> Signed-off-by: Konstantin Evtushenko <koevtushenko@...dex.com>
>> Signed-off-by: Sergey Bashirov <sergeybashirov@...il.com>
>> ---
>> Changes in v2:
>>  - Push down the check to layout driver level
>>
>>  fs/nfsd/blocklayout.c    | 8 +++++++-
>>  fs/nfsd/flexfilelayout.c | 2 +-
>>  fs/nfsd/nfs4proc.c       | 3 ++-
>>  fs/nfsd/pnfs.h           | 2 +-
>>  4 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfsd/blocklayout.c b/fs/nfsd/blocklayout.c
>> index 0822d8a119c6..1fbc5bbde07f 100644
>> --- a/fs/nfsd/blocklayout.c
>> +++ b/fs/nfsd/blocklayout.c
>> @@ -19,7 +19,7 @@
>>  
>>  static __be32
>>  nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp,
>> -		struct nfsd4_layoutget *args)
>> +		struct nfsd4_layoutget *args, bool in_grace)
>>  {
>>  	struct nfsd4_layout_seg *seg = &args->lg_seg;
>>  	struct super_block *sb = inode->i_sb;
>> @@ -34,6 +34,9 @@ nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp,
>>  		goto out_layoutunavailable;
>>  	}
>>  
>> +	if (in_grace)
>> +		goto out_grace;
> 
> Taste/style nit:
> 
> I prefer that the controlling svc_rqst is passed to ->proc_layoutget,
> rather than passing a boolean. The ff layout can just ignore that
> new parameter, and the block layout can deref the network namespace and
> do the locks_in_grace check.

Never mind. I will take v2 as is and fix this up myself.


>> +
>>  	/*
>>  	 * Some clients barf on non-zero block numbers for NONE or INVALID
>>  	 * layouts, so make sure to zero the whole structure.
>> @@ -111,6 +114,9 @@ nfsd4_block_proc_layoutget(struct inode *inode, const struct svc_fh *fhp,
>>  out_layoutunavailable:
>>  	seg->length = 0;
>>  	return nfserr_layoutunavailable;
>> +out_grace:
>> +	seg->length = 0;
>> +	return nfserr_grace;
>>  }
>>  
>>  static __be32
>> diff --git a/fs/nfsd/flexfilelayout.c b/fs/nfsd/flexfilelayout.c
>> index 3ca5304440ff..274a1e9bb596 100644
>> --- a/fs/nfsd/flexfilelayout.c
>> +++ b/fs/nfsd/flexfilelayout.c
>> @@ -21,7 +21,7 @@
>>  
>>  static __be32
>>  nfsd4_ff_proc_layoutget(struct inode *inode, const struct svc_fh *fhp,
>> -		struct nfsd4_layoutget *args)
>> +		struct nfsd4_layoutget *args, bool in_grace)
>>  {
>>  	struct nfsd4_layout_seg *seg = &args->lg_seg;
>>  	u32 device_generation = 0;
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index d7c58aa64f06..5d1d343a4e23 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -2435,6 +2435,7 @@ static __be32
>>  nfsd4_layoutget(struct svc_rqst *rqstp,
>>  		struct nfsd4_compound_state *cstate, union nfsd4_op_u *u)
>>  {
>> +	struct net *net = SVC_NET(rqstp);
>>  	struct nfsd4_layoutget *lgp = &u->layoutget;
>>  	struct svc_fh *current_fh = &cstate->current_fh;
>>  	const struct nfsd4_layout_ops *ops;
>> @@ -2498,7 +2499,7 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
>>  		goto out_put_stid;
>>  
>>  	nfserr = ops->proc_layoutget(d_inode(current_fh->fh_dentry),
>> -				     current_fh, lgp);
>> +				     current_fh, lgp, locks_in_grace(net));
>>  	if (nfserr)
>>  		goto out_put_stid;
>>  
>> diff --git a/fs/nfsd/pnfs.h b/fs/nfsd/pnfs.h
>> index dfd411d1f363..61c2528ef077 100644
>> --- a/fs/nfsd/pnfs.h
>> +++ b/fs/nfsd/pnfs.h
>> @@ -30,7 +30,7 @@ struct nfsd4_layout_ops {
>>  			const struct nfsd4_getdeviceinfo *gdevp);
>>  
>>  	__be32 (*proc_layoutget)(struct inode *, const struct svc_fh *fhp,
>> -			struct nfsd4_layoutget *lgp);
>> +			struct nfsd4_layoutget *lgp, bool in_grace);
>>  	__be32 (*encode_layoutget)(struct xdr_stream *xdr,
>>  			const struct nfsd4_layoutget *lgp);
>>  
> 
> 


-- 
Chuck Lever