On 9/5/23 05:31, David Laight wrote:
From: Mathieu Desnoyers
Sent: 04 September 2023 11:24
On 9/4/23 05:42, 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
The patch title should identify more precisely its context, e.g.:
"srcu: Fix srcu_struct node grpmask overflow on 64-bit systems"
Also, as I stated in my reply to the previous version, the patch commit
message should describe the impact of the bug it fixes.
And is the analysis complete?
Is 1UL right for 32bit archs??
Is 64 bits even enough??
I understand from include/linux/rcu_node_tree.h and kernel/rcu/Kconfig
RCU_FANOUT and RCU_FANOUT_LEAF ranges that a 32-bit integer is
sufficient to hold the mask on 32-bit architectures, and a 64-bit
integer is enough on 64-bit architectures given the current implementation.
At least this appears to be the intent. I did not do a thorough analysis
of the various parameter limits.
Thanks,
Mathieu
David
Thanks,
Mathieu
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@xxxxxxxxx>
---
v2: Added fixes to the srcu_schedule_cbs_snp function as suggested by
Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
kernel/rcu/srcutree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 20d7a238d675..6c18e6005ae1 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);
}
smp_store_release(&ssp->srcu_sup->srcu_size_state, SRCU_SIZE_WAIT_BARRIER);
return true;
@@ -833,7 +833,7 @@ static void srcu_schedule_cbs_snp(struct srcu_struct *ssp, struct srcu_node *snp
int cpu;
for (cpu = snp->grplo; cpu <= snp->grphi; cpu++) {
- if (!(mask & (1 << (cpu - snp->grplo))))
+ if (!(mask & (1UL << (cpu - snp->grplo))))
continue;
srcu_schedule_cbs_sdp(per_cpu_ptr(ssp->sda, cpu), delay);
}
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com
