linux-kernel - Re: [PATCH v2] NFSv4: prevent integer overflow while calling nfs4_set_lease

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <697350c4-f54a-4197-96f6-45ed53f08a4a@omp.ru>
Date: Fri, 25 Jul 2025 23:19:31 +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/2/25 7:47 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...

   Gentle ping!
   Anna, do you agree with this approach?

> [...]

MBR, Sergey