| 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
| ||
|
Message-ID: <1b374635-2444-453f-a7bd-7e732184998e@huawei.com>
Date: Thu, 7 Aug 2025 18:36:44 +0800
From: Wang Liang <wangliang74@...wei.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>, <idosch@...dia.com>,
<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <horms@...nel.org>
CC: <bridge@...ts.linux.dev>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <yuehaibing@...wei.com>,
<zhangchangzhong@...wei.com>
Subject: Re: [PATCH net] net: bridge: fix soft lockup in
br_multicast_query_expired()
在 2025/8/6 18:08, Nikolay Aleksandrov 写道:
> On 8/6/25 12:49, Wang Liang wrote:
>> When set multicast_query_interval to a large value, the local variable
>> 'time' in br_multicast_send_query() may overflow. If the time is smaller
>> than jiffies, the timer will expire immediately, and then call mod_timer()
>> again, which creates a loop and may trigger the following soft lockup
>> issue:
>>
>> watchdog: BUG: soft lockup - CPU#1 stuck for 221s! [rb_consumer:66]
>> CPU: 1 UID: 0 PID: 66 Comm: rb_consumer Not tainted 6.16.0+ #259 PREEMPT(none)
>> Call Trace:
>> <IRQ>
>> __netdev_alloc_skb+0x2e/0x3a0
>> br_ip6_multicast_alloc_query+0x212/0x1b70
>> __br_multicast_send_query+0x376/0xac0
>> br_multicast_send_query+0x299/0x510
>> br_multicast_query_expired.constprop.0+0x16d/0x1b0
>> call_timer_fn+0x3b/0x2a0
>> __run_timers+0x619/0x950
>> run_timer_softirq+0x11c/0x220
>> handle_softirqs+0x18e/0x560
>> __irq_exit_rcu+0x158/0x1a0
>> sysvec_apic_timer_interrupt+0x76/0x90
>> </IRQ>
>>
>> This issue can be reproduced with:
>> ip link add br0 type bridge
>> echo 1 > /sys/class/net/br0/bridge/multicast_querier
>> echo 0xffffffffffffffff >
>> /sys/class/net/br0/bridge/multicast_query_interval
>> ip link set dev br0 up
>>
>> Fix this by comparing expire time with jiffies, to avoid the timer loop.
>>
>> Fixes: 7e4df51eb35d ("bridge: netlink: add support for igmp's intervals")
>> Signed-off-by: Wang Liang <wangliang74@...wei.com>
>> ---
>> net/bridge/br_multicast.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 1377f31b719c..631ae3b4c45d 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -1892,7 +1892,8 @@ static void br_multicast_send_query(struct net_bridge_mcast *brmctx,
>> time += own_query->startup_sent < brmctx->multicast_startup_query_count ?
>> brmctx->multicast_startup_query_interval :
>> brmctx->multicast_query_interval;
>> - mod_timer(&own_query->timer, time);
>> + if (time_is_after_jiffies(time))
>> + mod_timer(&own_query->timer, time);
>> }
>>
>> static void
> This is the wrong way to fix it, it is a configuration issue, so we could either
> cap the value at something that noone uses, e.g. 24 hours, or we could make sure time
> is at least 1s (that is BR_MULTICAST_QUERY_INTVL_MIN).
>
> The simple fix would be to do a min(time, BR_MULTICAST_QUERY_INTVL_MIN), but I'd go
> for something similar to:
> commit 99b40610956a
> Author: Nikolay Aleksandrov <razor@...ckwall.org>
> Date: Mon Dec 27 19:21:15 2021 +0200
>
> net: bridge: mcast: add and enforce query interval minimum
>
> for the maximum to avoid the overflow altogether. By the way multicast_startup_query_interval
> would also cause the same issue, so you'd have to cap it.
>
> Cheers,
> Nik
Thanks very much for your suggestions!
Similar to the commit 99b40610956a("net: bridge: mcast: add and enforce
query interval minimum"), it is indeed a better choice to add query
interval maximum:
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 1377f31b719c..8ce145938b02 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -4818,6 +4818,14 @@ void br_multicast_set_query_intvl(struct
net_bridge_mcast *brmctx,
intvl_jiffies = BR_MULTICAST_QUERY_INTVL_MIN;
}
+ if (intvl_jiffies > BR_MULTICAST_QUERY_INTVL_MAX) {
+ br_info(brmctx->br,
+ "trying to set multicast query interval above
maximum, setting to %lu (%ums)\n",
+ jiffies_to_clock_t(BR_MULTICAST_QUERY_INTVL_MAX),
+ jiffies_to_msecs(BR_MULTICAST_QUERY_INTVL_MAX));
+ intvl_jiffies = BR_MULTICAST_QUERY_INTVL_MAX;
+ }
+
brmctx->multicast_query_interval = intvl_jiffies;
}
@@ -4834,6 +4842,14 @@ void br_multicast_set_startup_query_intvl(struct
net_bridge_mcast *brmctx,
intvl_jiffies = BR_MULTICAST_STARTUP_QUERY_INTVL_MIN;
}
+ if (intvl_jiffies > BR_MULTICAST_STARTUP_QUERY_INTVL_MAX) {
+ br_info(brmctx->br,
+ "trying to set multicast startup query interval
above maximum, setting to %lu (%ums)\n",
+ jiffies_to_clock_t(BR_MULTICAST_STARTUP_QUERY_INTVL_MAX),
+ jiffies_to_msecs(BR_MULTICAST_STARTUP_QUERY_INTVL_MAX));
+ intvl_jiffies = BR_MULTICAST_STARTUP_QUERY_INTVL_MAX;
+ }
+
brmctx->multicast_startup_query_interval = intvl_jiffies;
}
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index b159aae594c0..8de0904b9627 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -31,6 +31,8 @@
#define BR_MULTICAST_DEFAULT_HASH_MAX 4096
#define BR_MULTICAST_QUERY_INTVL_MIN msecs_to_jiffies(1000)
#define BR_MULTICAST_STARTUP_QUERY_INTVL_MIN BR_MULTICAST_QUERY_INTVL_MIN
+#define BR_MULTICAST_QUERY_INTVL_MAX msecs_to_jiffies(86400000) /* 24
hours */
+#define BR_MULTICAST_STARTUP_QUERY_INTVL_MAX BR_MULTICAST_QUERY_INTVL_MAX
#define BR_HWDOM_MAX BITS_PER_LONG
Set the interval maximum to 24 hours, it is big enough.
Please check it. If the above code is ok, I will send a v2 patch then.
------
Best regards
Wang Liang
>
>
Powered by blists - more mailing lists