[PATCH v3 1/3] nfsd: reject out-of-range useconds in NFSv2 SETATTR/CREATE
robbieko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]>
| Newsgroups | gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
From: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]> The NFSv2 sattr decoder converts the wire useconds to nanoseconds in svcxdr_decode_sattr(): iap->ia_atime.tv_nsec = tmp2 * NSEC_PER_USEC; tmp2 is a u32 and NSEC_PER_USEC is 1000, so the product is computed in unsigned long. On ILP32 that is 32 bits, and an out-of-range useconds value such as 4294968 wraps to tv_nsec == 704. The corruption therefore happens during decode, before any proc function can inspect the value, and a later range check on tv_nsec would see an in-range result and accept it. Guard the raw useconds before the multiplication and reject values greater than 1000000. useconds == 1000000 is kept: it is the Sun convention for "set to the current server time", and the in-tree Linux NFSv2 client emits it in both the atime and the mtime field for a plain touch / utimes(file, NULL) (see encode_sattr() and xdr_encode_current_server_time() in fs/nfs/nfs2xdr.c). Rejecting 1000000 would turn that common operation into a hard decode failure for both SETATTR and CREATE. 1000000 * NSEC_PER_USEC is 10^9, which does not wrap on ILP32, so the Sun convention value passes through safely; only genuinely out-of-range values (> 1000000) are rejected. The atime and mtime guards are therefore symmetric. The decoder only applied the Sun convention in the mtime block, which clears ATTR_ATIME_SET|ATTR_MTIME_SET when mtime useconds == 1000000. If a client puts 1000000 in the atime field but not in the mtime field, the atime block stored an out-of-range tv_nsec (10^9) and left ATTR_ATIME_SET set, so the bogus value reached the filesystem. Apply the convention in the atime block as well, clearing ATTR_ATIME_SET so the server uses its current time and ignores the value. Only ATTR_ATIME_SET is cleared there; the mtime block keeps its existing behavior, where 1000000 means "set both atime and mtime to now". Signed-off-by: Robbie Ko <robbieko-UelDjCVBxVpWk0Htik3J/[email protected]> --- fs/nfsd/nfsxdr.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c index ae71e0621317..48a4e89a5f41 100644 --- a/fs/nfsd/nfsxdr.c +++ b/fs/nfsd/nfsxdr.c @@ -172,14 +172,45 @@ svcxdr_decode_sattr(struct svc_rqst *rqstp, struct xdr_stream *xdr, tmp1 = be32_to_cpup(p++); tmp2 = be32_to_cpup(p++); if (tmp1 != (u32)-1 && tmp2 != (u32)-1) { + /* + * Guard the raw useconds before the unit conversion below. + * tmp2 * NSEC_PER_USEC is computed in unsigned long, which is + * 32 bits on ILP32, so an out-of-range value would wrap and + * silently produce a bogus in-range tv_nsec. useconds == + * 1000000 is the Sun "set to current server time" convention + * (see the mtime block below); allow it and reject anything + * larger. Note 1000000 * NSEC_PER_USEC is 10^9, which does not + * wrap on ILP32. + */ + if (tmp2 > 1000000) + return false; iap->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET; iap->ia_atime.tv_sec = tmp1; iap->ia_atime.tv_nsec = tmp2 * NSEC_PER_USEC; + /* + * The Linux NFSv2 client emits useconds == 1000000 in the + * atime field too (touch / utimes(file, NULL) sets ATTR_ATIME + * without ATTR_ATIME_SET). Apply the Sun convention here so + * the server uses its current time and ignores the bogus + * tv_nsec, instead of storing an out-of-range value when the + * mtime field does not also carry 1000000. Only ATTR_ATIME_SET + * is cleared; the mtime block keeps its own handling, where + * 1000000 means "set both atime and mtime to now". + */ + if (tmp2 == 1000000) + iap->ia_valid &= ~ATTR_ATIME_SET; } tmp1 = be32_to_cpup(p++); tmp2 = be32_to_cpup(p++); if (tmp1 != (u32)-1 && tmp2 != (u32)-1) { + /* + * useconds == 1000000 is a valid Sun convention here (see + * below); anything above that is out of range. Guard it before + * the unit conversion to avoid the ILP32 wrap described above. + */ + if (tmp2 > 1000000) + return false; iap->ia_valid |= ATTR_MTIME | ATTR_MTIME_SET; iap->ia_mtime.tv_sec = tmp1; iap->ia_mtime.tv_nsec = tmp2 * NSEC_PER_USEC; -- 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.