Re: [PATCH] nfsd: reject out-of-range nseconds in NFSv3 nfstime3 decode

On Thu, 2026-06-11 at 12:09 +0800, robbieko wrote:
> From: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]>
> 
> The NFSv3 nfstime3 decoder svcxdr_decode_nfstime3() accepts the 32-bit
> nseconds field from the wire without validating its range. RFC 1813
> does not constrain the value, but a tv_nsec >= NSEC_PER_SEC is not a
> valid timespec64. The NFSv4 decoder nfsd4_decode_nfstime4() already
> rejects such values with NFS4ERR_INVAL; NFSv3 had no equivalent check.
> 
> A malicious or buggy client can therefore send a SETATTR carrying an
> out-of-range nseconds (up to 4294967295). The value flows through
> nfsd_setattr() -> notify_change() -> timestamp_truncate(), which does
> not clamp tv_nsec to < NSEC_PER_SEC when the filesystem supports
> nanosecond granularity (s_time_gran == 1). The inode atime/mtime
> setters store it verbatim (only ctime is normalized via
> inode_set_ctime_to_ts()).
> 
> The un-normalized value then corrupts on-disk metadata: ext4's
> ext4_encode_extra_time() shifts tv_nsec left by EXT4_EPOCH_BITS, which
> overflows the 32-bit extra field and clobbers the seconds-epoch bits,
> so the stored seconds (year) are wrong on read-back. XFS with bigtime
> mis-stores the timestamp for the same reason. This is silent, with no
> WARN_ON anywhere in the path to catch it.
> 
> Validate the decoded nseconds in svcxdr_decode_nfstime3() and fail the
> XDR decode if it is out of range, mirroring the NFSv4 behavior.
> 
> Signed-off-by: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]>
> ---
>  fs/nfsd/nfs3xdr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 2ff9a991a8fb..59e82ce03c20 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -64,6 +64,8 @@ svcxdr_decode_nfstime3(struct xdr_stream *xdr, struct timespec64 *timep)
>  		return false;
>  	timep->tv_sec = be32_to_cpup(p++);
>  	timep->tv_nsec = be32_to_cpup(p);
> +	if (timep->tv_nsec >= (u32)1000000000)
> +		return false;
>  
>  	return true;
>  }

Conceptually this looks fine, but it should use the named constant
(NSEC_PER_SEC). Also, does v2 have a similar issue?
-- 
Jeff Layton <[email protected]>