[lvc-project] Re: [PATCH] nfsd: fix arithmetic expression overflow in decode_saddr()

[NeilBrown]

On Fri, 26 Sep 2025, Alexandr Sapozhnkiov wrote:
> From: Alexandr Sapozhnikov <alsp705 at gmail.com>
> 
> The value of an arithmetic expression tmp2 * NSEC_PER_USEC 
> is a subject to overflow because its operands are not cast 
> to a larger data type before performing arithmetic.
> If tmp2 == 17,000,000 then the expression tmp2 * NSEC_PER_USEC
> will overflow because expression is of type u32.
> If tmp2 > 1,000,000 then tv_nsec will give be greater 
> than 1 second.

You didn't answer my question: why should we be bothered by an over
flow? What harm does it cause?

If we are going to bother detecting an incorrect value, we should
respond to it by returning false.  That would tell the client that the
numbers it provided weren't a valid time.



> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Alexandr Sapozhnikov <alsp705 at gmail.com>
> ---
>  fs/nfsd/nfsxdr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
> index 5777f40c7353..df62ed5099de 100644
> --- a/fs/nfsd/nfsxdr.c
> +++ b/fs/nfsd/nfsxdr.c
> @@ -172,6 +172,8 @@ 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) {
> +		if (tmp2 > 999999)
> +			tmp2 = 999999;
>  		iap->ia_valid |= ATTR_ATIME | ATTR_ATIME_SET;
>  		iap->ia_atime.tv_sec = tmp1;
>  		iap->ia_atime.tv_nsec = tmp2 * NSEC_PER_USEC;
> @@ -180,6 +182,8 @@ 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) {
> +		if (tmp2 > 1000000)
> +			tmp2 = 999999;

Why are the two code fragments different?  This isn't an AI writing the
patch is it?

NeilBrown

>  		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
> 
> 
>