[PATCH v2 1/2] nfsd: reject out-of-range nseconds in setattr atime/mtime

From: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]>

A client can send a SETATTR (and, for NFSv3, a file creation) carrying
an atime or mtime whose nseconds field is out of range. The value is
well-formed on the wire and decodes cleanly into a valid uint32, but it
is not a valid timespec64: tv_nsec must be less than NSEC_PER_SEC.

Nothing in the path clamps it. notify_change() runs the time through
timestamp_truncate(), which does not reduce tv_nsec below NSEC_PER_SEC
when the filesystem supports nanosecond granularity (s_time_gran == 1),
and 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; an
out-of-range value overflows the 32-bit extra field and clobbers the
seconds-epoch bits, so the stored seconds (and thus the year) are wrong
on read-back. XFS with bigtime mis-stores the timestamp for the same
reason. There is no WARN_ON anywhere in the path to catch it.

Validate the client-supplied atime/mtime in nfsd_setattr(), which is
the common choke point for SETATTR and for the create paths (via
nfsd_create_setattr()), and covers both NFSv2 and NFSv3. The check is
done up front, before any resources are acquired, so no cleanup path is
involved; RFC 1813 Section 2.6 leaves error precedence to the
implementation. Return NFS3ERR_INVAL, which RFC 1813 lists for SETATTR
and describes as the error for a value the server 'can not store ... in
its own representation'. The client maps this to EINVAL.

tv_nsec is a long, so the comparison casts it to unsigned long (the same
width) rather than to u32, matching timespec64_valid(). A u32 cast would
be wrong on both ends: on 32-bit it cannot widen, and on 64-bit it would
truncate the NFSv2 value (svcxdr_decode_sattr() computes tv_nsec as
tmp2 * NSEC_PER_USEC, which can exceed the u32 range). The unsigned long
cast also rejects a value that became negative when an out-of-range u32
wire nseconds was assigned to a 32-bit long.

RFC 1094 does not define NFSERR_INVAL for NFSv2; its stat enum has no
value 22. Map nfserr_inval to nfserr_io in nfsd_map_status() so the
NFSv2 reply stays within the RFC 1094 status set, the same way that
function already folds other internal statuses with no NFSv2 equivalent.
NFSv3 (and NFSv4) leave the INVAL status as is, since it is valid there.

Only client-supplied times are checked: SET_TO_SERVER_TIME requests
carry no client value and are filled in by the server. The NFSv2 Sun
'set both to now' convention clears ATTR_[AM]TIME_SET in the SETATTR
proc before nfsd_setattr() runs, so it is unaffected. The sattrguard3
ctime is deliberately left alone: an out-of-range guard simply never
matches the object's ctime and yields NFS3ERR_NOT_SYNC via the existing
guardtime comparison, which is the protocol-correct outcome rather than
rejecting the request.

NFSv4 already rejects such values in nfsd4_decode_nfstime4(), so they do
not reach this check on that path.

The lack of validation is long-standing and predates the git history of
this code, so no Fixes: tag is provided. This is a data-integrity fix
and is a candidate for LTS backport.

Signed-off-by: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]>
---
 fs/nfsd/nfsproc.c |  1 +
 fs/nfsd/vfs.c     | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c
index 8873033d1e82..3c8da3f1af6c 100644
--- a/fs/nfsd/nfsproc.c
+++ b/fs/nfsd/nfsproc.c
@@ -33,6 +33,7 @@ static __be32 nfsd_map_status(__be32 status)
 		break;
 	case nfserr_symlink:
 	case nfserr_wrong_type:
+	case nfserr_inval:
 		status = nfserr_io;
 		break;
 	}
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index eafdf7b7890f..dd0bbf7aad1b 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -515,6 +515,20 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 	trace_nfsd_vfs_setattr(rqstp, fhp, iap, guardtime);
 
+	/*
+	 * Reject a client-supplied atime or mtime whose tv_nsec is out of
+	 * range. Such a value is well-formed on the wire but is not a valid
+	 * timespec64; storing it verbatim can corrupt on-disk timestamps
+	 * (for example, ext4 packs tv_nsec << 2 alongside epoch bits).
+	 * Reject it before acquiring any resources. RFC 1813 Section 2.6
+	 * leaves error precedence to the implementation.
+	 */
+	if (((iap->ia_valid & ATTR_ATIME_SET) &&
+	     (unsigned long)iap->ia_atime.tv_nsec >= NSEC_PER_SEC) ||
+	    ((iap->ia_valid & ATTR_MTIME_SET) &&
+	     (unsigned long)iap->ia_mtime.tv_nsec >= NSEC_PER_SEC))
+		return nfserr_inval;
+
 	if (iap->ia_valid & ATTR_SIZE) {
 		accmode |= NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE;
 		ftype = S_IFREG;
-- 
2.43.0


Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.