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: <8244b3f4-ea69-46ee-9ba2-6711f516f00b@omp.ru>
Date: Wed, 2 Jul 2025 19:47:36 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: <linux-nfs@...r.kernel.org>, Trond Myklebust <trondmy@...nel.org>, Anna
 Schumaker <anna@...nel.org>
CC: <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] NFSv4: prevent integer overflow while calling
 nfs4_set_lease_period()

On 7/1/25 11:35 PM, Sergey Shtylyov wrote:

> The nfs_client::cl_lease_time field (as well as the jiffies variable it's
> used with) is declared as *unsigned long*, which is 32-bit type on 32-bit
> arches and 64-bit type on 64-bit arches. When nfs4_set_lease_period() that
> sets nfs_client::cl_lease_time is called, 32-bit nfs_fsinfo::lease_time
> field is multiplied by HZ -- that might overflow before being implicitly
> cast to *unsigned long*. Actually, there's no need to multiply by HZ at all
> the call sites of nfs4_set_lease_period() -- it makes more sense to do that
> once, inside that function, calling check_mul_overflow() and capping result
> at ULONG_MAX on actual overflow...
> 
> Found by Linux Verification Center (linuxtesting.org) with the Svace static
> analysis tool.
> 
> Signed-off-by: Sergey Shtylyov <s.shtylyov@....ru>
> Cc: stable@...r.kernel.org
> 
> ---
> The patch is against the master branch of Trond Myklebust's linux-nfs.git repo.
> 
> Changes in version 2:
> - made use of check_mul_overflow() instead of mul_u32_u32();
> - capped the multiplication result at ULONG_MAX instead of returning -ERANGE,
>   keeping nfs4_set_lease_period() *void*;
> - rewrote the patch description accordingly.

   Forgot to say that I had to adjust the patch description to make it clear
that the overflow happens on 64-bit arches as well...

[...]

MBR, Sergey


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ