linux-kernel - Re: [PATCH] srcu: The value may overflow

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d05deed9-637f-697c-5c2f-d6fede4c956d@efficios.com>
Date:   Fri, 1 Sep 2023 09:31:58 -0400
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Denis Arefev <arefev@...mel.ru>,
        Lai Jiangshan <jiangshanlai@...il.com>
Cc:     "Paul E. McKenney" <paulmck@...nel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>, rcu@...r.kernel.org,
        lvc-project@...uxtesting.org, linux-kernel@...r.kernel.org,
        trufanov@...mel.ru, vfh@...mel.ru
Subject: Re: [PATCH] srcu: The value may overflow

On 9/1/23 05:53, Denis Arefev wrote:
> The value of an arithmetic expression 1 << (cpu - sdp->mynode->grplo)
> is subject to overflow due to a failure to cast operands to a larger
> data type before performing arithmetic
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Denis Arefev <arefev@...mel.ru>
> ---
>   kernel/rcu/srcutree.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 20d7a238d675..e14b74fb1ba0 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -223,7 +223,7 @@ static bool init_srcu_struct_nodes(struct srcu_struct *ssp, gfp_t gfp_flags)
>   				snp->grplo = cpu;
>   			snp->grphi = cpu;
>   		}
> -		sdp->grpmask = 1 << (cpu - sdp->mynode->grplo);
> +		sdp->grpmask = 1UL << (cpu - sdp->mynode->grplo);

What possible values of cpus supported by the Linux kernel and grplo can 
cause this to overflow on 64-bit architectures ? I suspect the maximum 
result of this subtraction is defined by the RCU_FANOUT or other srcu 
level-spread values assigned by rcu_init_levelspread(), which can indeed 
cause the signed 32-bit integer literal ("1") to overflow when shifted 
by any value greater than 31. This analysis should be added to the 
commit message so the impact of the issue can be understood.

I also notice this in the same file:

srcu_schedule_cbs_snp():

         for (cpu = snp->grplo; cpu <= snp->grphi; cpu++) {
                 if (!(mask & (1 << (cpu - snp->grplo))))
                         continue;

Which should be fixed at the same time.

Thanks,

Mathieu

>   	}
>   	smp_store_release(&ssp->srcu_sup->srcu_size_state, SRCU_SIZE_WAIT_BARRIER);
>   	return true;

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com