* [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire()
@ 2021-08-10 9:45 Eric Dumazet
2021-08-10 11:00 ` patchwork-bot+netdevbpf
2021-08-11 18:29 ` Guenter Roeck
0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2021-08-10 9:45 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: netdev, Eric Dumazet, Eric Dumazet, syzbot
From: Eric Dumazet <edumazet@google.com>
Fix the data-race reported by syzbot [1]
Issue here is that igmp_ifc_timer_expire() can update in_dev->mr_ifc_count
while another change just occured from another context.
in_dev->mr_ifc_count is only 8bit wide, so the race had little
consequences.
[1]
BUG: KCSAN: data-race in igmp_ifc_event / igmp_ifc_timer_expire
write to 0xffff8881051e3062 of 1 bytes by task 12547 on cpu 0:
igmp_ifc_event+0x1d5/0x290 net/ipv4/igmp.c:821
igmp_group_added+0x462/0x490 net/ipv4/igmp.c:1356
____ip_mc_inc_group+0x3ff/0x500 net/ipv4/igmp.c:1461
__ip_mc_join_group+0x24d/0x2c0 net/ipv4/igmp.c:2199
ip_mc_join_group_ssm+0x20/0x30 net/ipv4/igmp.c:2218
do_ip_setsockopt net/ipv4/ip_sockglue.c:1285 [inline]
ip_setsockopt+0x1827/0x2a80 net/ipv4/ip_sockglue.c:1423
tcp_setsockopt+0x8c/0xa0 net/ipv4/tcp.c:3657
sock_common_setsockopt+0x5d/0x70 net/core/sock.c:3362
__sys_setsockopt+0x18f/0x200 net/socket.c:2159
__do_sys_setsockopt net/socket.c:2170 [inline]
__se_sys_setsockopt net/socket.c:2167 [inline]
__x64_sys_setsockopt+0x62/0x70 net/socket.c:2167
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
read to 0xffff8881051e3062 of 1 bytes by interrupt on cpu 1:
igmp_ifc_timer_expire+0x706/0xa30 net/ipv4/igmp.c:808
call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1419
expire_timers+0x135/0x250 kernel/time/timer.c:1464
__run_timers+0x358/0x420 kernel/time/timer.c:1732
run_timer_softirq+0x19/0x30 kernel/time/timer.c:1745
__do_softirq+0x12c/0x26e kernel/softirq.c:558
invoke_softirq kernel/softirq.c:432 [inline]
__irq_exit_rcu+0x9a/0xb0 kernel/softirq.c:636
sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1100
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:638
console_unlock+0x8e8/0xb30 kernel/printk/printk.c:2646
vprintk_emit+0x125/0x3d0 kernel/printk/printk.c:2174
vprintk_default+0x22/0x30 kernel/printk/printk.c:2185
vprintk+0x15a/0x170 kernel/printk/printk_safe.c:392
printk+0x62/0x87 kernel/printk/printk.c:2216
selinux_netlink_send+0x399/0x400 security/selinux/hooks.c:6041
security_netlink_send+0x42/0x90 security/security.c:2070
netlink_sendmsg+0x59e/0x7c0 net/netlink/af_netlink.c:1919
sock_sendmsg_nosec net/socket.c:703 [inline]
sock_sendmsg net/socket.c:723 [inline]
____sys_sendmsg+0x360/0x4d0 net/socket.c:2392
___sys_sendmsg net/socket.c:2446 [inline]
__sys_sendmsg+0x1ed/0x270 net/socket.c:2475
__do_sys_sendmsg net/socket.c:2484 [inline]
__se_sys_sendmsg net/socket.c:2482 [inline]
__x64_sys_sendmsg+0x42/0x50 net/socket.c:2482
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
value changed: 0x01 -> 0x02
Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 12539 Comm: syz-executor.1 Not tainted 5.14.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/ipv4/igmp.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 6b3c558a4f232652b97a078d48f302864e60a866..a51360087b19845a28408c827032e08dabf99838 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -803,10 +803,17 @@ static void igmp_gq_timer_expire(struct timer_list *t)
static void igmp_ifc_timer_expire(struct timer_list *t)
{
struct in_device *in_dev = from_timer(in_dev, t, mr_ifc_timer);
+ u8 mr_ifc_count;
igmpv3_send_cr(in_dev);
- if (in_dev->mr_ifc_count) {
- in_dev->mr_ifc_count--;
+restart:
+ mr_ifc_count = READ_ONCE(in_dev->mr_ifc_count);
+
+ if (mr_ifc_count) {
+ if (cmpxchg(&in_dev->mr_ifc_count,
+ mr_ifc_count,
+ mr_ifc_count - 1) != mr_ifc_count)
+ goto restart;
igmp_ifc_start_timer(in_dev,
unsolicited_report_interval(in_dev));
}
@@ -818,7 +825,7 @@ static void igmp_ifc_event(struct in_device *in_dev)
struct net *net = dev_net(in_dev->dev);
if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
return;
- in_dev->mr_ifc_count = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
+ WRITE_ONCE(in_dev->mr_ifc_count, in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv);
igmp_ifc_start_timer(in_dev, 1);
}
@@ -957,7 +964,7 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
in_dev->mr_qri;
}
/* cancel the interface change timer */
- in_dev->mr_ifc_count = 0;
+ WRITE_ONCE(in_dev->mr_ifc_count, 0);
if (del_timer(&in_dev->mr_ifc_timer))
__in_dev_put(in_dev);
/* clear deleted report items */
@@ -1724,7 +1731,7 @@ void ip_mc_down(struct in_device *in_dev)
igmp_group_dropped(pmc);
#ifdef CONFIG_IP_MULTICAST
- in_dev->mr_ifc_count = 0;
+ WRITE_ONCE(in_dev->mr_ifc_count, 0);
if (del_timer(&in_dev->mr_ifc_timer))
__in_dev_put(in_dev);
in_dev->mr_gq_running = 0;
@@ -1941,7 +1948,7 @@ static int ip_mc_del_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
pmc->sfmode = MCAST_INCLUDE;
#ifdef CONFIG_IP_MULTICAST
pmc->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
- in_dev->mr_ifc_count = pmc->crcount;
+ WRITE_ONCE(in_dev->mr_ifc_count, pmc->crcount);
for (psf = pmc->sources; psf; psf = psf->sf_next)
psf->sf_crcount = 0;
igmp_ifc_event(pmc->interface);
@@ -2120,7 +2127,7 @@ static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
/* else no filters; keep old mode for reports */
pmc->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
- in_dev->mr_ifc_count = pmc->crcount;
+ WRITE_ONCE(in_dev->mr_ifc_count, pmc->crcount);
for (psf = pmc->sources; psf; psf = psf->sf_next)
psf->sf_crcount = 0;
igmp_ifc_event(in_dev);
--
2.32.0.605.g8dce9f2422-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire()
2021-08-10 9:45 [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire() Eric Dumazet
@ 2021-08-10 11:00 ` patchwork-bot+netdevbpf
2021-08-11 18:29 ` Guenter Roeck
1 sibling, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-08-10 11:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, kuba, netdev, edumazet, syzkaller
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Tue, 10 Aug 2021 02:45:47 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Fix the data-race reported by syzbot [1]
> Issue here is that igmp_ifc_timer_expire() can update in_dev->mr_ifc_count
> while another change just occured from another context.
>
> in_dev->mr_ifc_count is only 8bit wide, so the race had little
> consequences.
>
> [...]
Here is the summary with links:
- [net] net: igmp: fix data-race in igmp_ifc_timer_expire()
https://git.kernel.org/netdev/net/c/4a2b285e7e10
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire()
2021-08-10 9:45 [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire() Eric Dumazet
2021-08-10 11:00 ` patchwork-bot+netdevbpf
@ 2021-08-11 18:29 ` Guenter Roeck
2021-08-11 18:32 ` Guenter Roeck
1 sibling, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-08-11 18:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, netdev, Eric Dumazet, syzbot
On Tue, Aug 10, 2021 at 02:45:47AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Fix the data-race reported by syzbot [1]
> Issue here is that igmp_ifc_timer_expire() can update in_dev->mr_ifc_count
> while another change just occured from another context.
>
> in_dev->mr_ifc_count is only 8bit wide, so the race had little
> consequences.
>
> [1]
> BUG: KCSAN: data-race in igmp_ifc_event / igmp_ifc_timer_expire
>
> write to 0xffff8881051e3062 of 1 bytes by task 12547 on cpu 0:
> igmp_ifc_event+0x1d5/0x290 net/ipv4/igmp.c:821
> igmp_group_added+0x462/0x490 net/ipv4/igmp.c:1356
> ____ip_mc_inc_group+0x3ff/0x500 net/ipv4/igmp.c:1461
> __ip_mc_join_group+0x24d/0x2c0 net/ipv4/igmp.c:2199
> ip_mc_join_group_ssm+0x20/0x30 net/ipv4/igmp.c:2218
> do_ip_setsockopt net/ipv4/ip_sockglue.c:1285 [inline]
> ip_setsockopt+0x1827/0x2a80 net/ipv4/ip_sockglue.c:1423
> tcp_setsockopt+0x8c/0xa0 net/ipv4/tcp.c:3657
> sock_common_setsockopt+0x5d/0x70 net/core/sock.c:3362
> __sys_setsockopt+0x18f/0x200 net/socket.c:2159
> __do_sys_setsockopt net/socket.c:2170 [inline]
> __se_sys_setsockopt net/socket.c:2167 [inline]
> __x64_sys_setsockopt+0x62/0x70 net/socket.c:2167
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> read to 0xffff8881051e3062 of 1 bytes by interrupt on cpu 1:
> igmp_ifc_timer_expire+0x706/0xa30 net/ipv4/igmp.c:808
> call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1419
> expire_timers+0x135/0x250 kernel/time/timer.c:1464
> __run_timers+0x358/0x420 kernel/time/timer.c:1732
> run_timer_softirq+0x19/0x30 kernel/time/timer.c:1745
> __do_softirq+0x12c/0x26e kernel/softirq.c:558
> invoke_softirq kernel/softirq.c:432 [inline]
> __irq_exit_rcu+0x9a/0xb0 kernel/softirq.c:636
> sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1100
> asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:638
> console_unlock+0x8e8/0xb30 kernel/printk/printk.c:2646
> vprintk_emit+0x125/0x3d0 kernel/printk/printk.c:2174
> vprintk_default+0x22/0x30 kernel/printk/printk.c:2185
> vprintk+0x15a/0x170 kernel/printk/printk_safe.c:392
> printk+0x62/0x87 kernel/printk/printk.c:2216
> selinux_netlink_send+0x399/0x400 security/selinux/hooks.c:6041
> security_netlink_send+0x42/0x90 security/security.c:2070
> netlink_sendmsg+0x59e/0x7c0 net/netlink/af_netlink.c:1919
> sock_sendmsg_nosec net/socket.c:703 [inline]
> sock_sendmsg net/socket.c:723 [inline]
> ____sys_sendmsg+0x360/0x4d0 net/socket.c:2392
> ___sys_sendmsg net/socket.c:2446 [inline]
> __sys_sendmsg+0x1ed/0x270 net/socket.c:2475
> __do_sys_sendmsg net/socket.c:2484 [inline]
> __se_sys_sendmsg net/socket.c:2482 [inline]
> __x64_sys_sendmsg+0x42/0x50 net/socket.c:2482
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> value changed: 0x01 -> 0x02
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 1 PID: 12539 Comm: syz-executor.1 Not tainted 5.14.0-rc4-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> ---
> net/ipv4/igmp.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 6b3c558a4f232652b97a078d48f302864e60a866..a51360087b19845a28408c827032e08dabf99838 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -803,10 +803,17 @@ static void igmp_gq_timer_expire(struct timer_list *t)
> static void igmp_ifc_timer_expire(struct timer_list *t)
> {
> struct in_device *in_dev = from_timer(in_dev, t, mr_ifc_timer);
> + u8 mr_ifc_count;
>
> igmpv3_send_cr(in_dev);
> - if (in_dev->mr_ifc_count) {
> - in_dev->mr_ifc_count--;
> +restart:
> + mr_ifc_count = READ_ONCE(in_dev->mr_ifc_count);
> +
> + if (mr_ifc_count) {
> + if (cmpxchg(&in_dev->mr_ifc_count,
Unfortunately riscv only supports 4-byte and 8-byte cmpxchg(),
so this results in build errors.
Building riscv:defconfig ... failed
--------------
Error log:
In file included from <command-line>:
net/ipv4/igmp.c: In function 'igmp_ifc_timer_expire':
include/linux/compiler_types.h:328:45: error: call to '__compiletime_assert_547' declared with attribute error: BUILD_BUG failed
Guenter
> + mr_ifc_count,
> + mr_ifc_count - 1) != mr_ifc_count)
> + goto restart;
> igmp_ifc_start_timer(in_dev,
> unsolicited_report_interval(in_dev));
> }
> @@ -818,7 +825,7 @@ static void igmp_ifc_event(struct in_device *in_dev)
> struct net *net = dev_net(in_dev->dev);
> if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev))
> return;
> - in_dev->mr_ifc_count = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
> + WRITE_ONCE(in_dev->mr_ifc_count, in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv);
> igmp_ifc_start_timer(in_dev, 1);
> }
>
> @@ -957,7 +964,7 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
> in_dev->mr_qri;
> }
> /* cancel the interface change timer */
> - in_dev->mr_ifc_count = 0;
> + WRITE_ONCE(in_dev->mr_ifc_count, 0);
> if (del_timer(&in_dev->mr_ifc_timer))
> __in_dev_put(in_dev);
> /* clear deleted report items */
> @@ -1724,7 +1731,7 @@ void ip_mc_down(struct in_device *in_dev)
> igmp_group_dropped(pmc);
>
> #ifdef CONFIG_IP_MULTICAST
> - in_dev->mr_ifc_count = 0;
> + WRITE_ONCE(in_dev->mr_ifc_count, 0);
> if (del_timer(&in_dev->mr_ifc_timer))
> __in_dev_put(in_dev);
> in_dev->mr_gq_running = 0;
> @@ -1941,7 +1948,7 @@ static int ip_mc_del_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
> pmc->sfmode = MCAST_INCLUDE;
> #ifdef CONFIG_IP_MULTICAST
> pmc->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
> - in_dev->mr_ifc_count = pmc->crcount;
> + WRITE_ONCE(in_dev->mr_ifc_count, pmc->crcount);
> for (psf = pmc->sources; psf; psf = psf->sf_next)
> psf->sf_crcount = 0;
> igmp_ifc_event(pmc->interface);
> @@ -2120,7 +2127,7 @@ static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
> /* else no filters; keep old mode for reports */
>
> pmc->crcount = in_dev->mr_qrv ?: net->ipv4.sysctl_igmp_qrv;
> - in_dev->mr_ifc_count = pmc->crcount;
> + WRITE_ONCE(in_dev->mr_ifc_count, pmc->crcount);
> for (psf = pmc->sources; psf; psf = psf->sf_next)
> psf->sf_crcount = 0;
> igmp_ifc_event(in_dev);
> --
> 2.32.0.605.g8dce9f2422-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire()
2021-08-11 18:29 ` Guenter Roeck
@ 2021-08-11 18:32 ` Guenter Roeck
2021-08-11 19:45 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-08-11 18:32 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Jakub Kicinski, netdev, Eric Dumazet, syzbot
On Wed, Aug 11, 2021 at 11:29:25AM -0700, Guenter Roeck wrote:
> On Tue, Aug 10, 2021 at 02:45:47AM -0700, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Fix the data-race reported by syzbot [1]
> > Issue here is that igmp_ifc_timer_expire() can update in_dev->mr_ifc_count
> > while another change just occured from another context.
> >
> > in_dev->mr_ifc_count is only 8bit wide, so the race had little
> > consequences.
> >
> > [1]
> > BUG: KCSAN: data-race in igmp_ifc_event / igmp_ifc_timer_expire
> >
> > write to 0xffff8881051e3062 of 1 bytes by task 12547 on cpu 0:
> > igmp_ifc_event+0x1d5/0x290 net/ipv4/igmp.c:821
> > igmp_group_added+0x462/0x490 net/ipv4/igmp.c:1356
> > ____ip_mc_inc_group+0x3ff/0x500 net/ipv4/igmp.c:1461
> > __ip_mc_join_group+0x24d/0x2c0 net/ipv4/igmp.c:2199
> > ip_mc_join_group_ssm+0x20/0x30 net/ipv4/igmp.c:2218
> > do_ip_setsockopt net/ipv4/ip_sockglue.c:1285 [inline]
> > ip_setsockopt+0x1827/0x2a80 net/ipv4/ip_sockglue.c:1423
> > tcp_setsockopt+0x8c/0xa0 net/ipv4/tcp.c:3657
> > sock_common_setsockopt+0x5d/0x70 net/core/sock.c:3362
> > __sys_setsockopt+0x18f/0x200 net/socket.c:2159
> > __do_sys_setsockopt net/socket.c:2170 [inline]
> > __se_sys_setsockopt net/socket.c:2167 [inline]
> > __x64_sys_setsockopt+0x62/0x70 net/socket.c:2167
> > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> > entry_SYSCALL_64_after_hwframe+0x44/0xae
> >
> > read to 0xffff8881051e3062 of 1 bytes by interrupt on cpu 1:
> > igmp_ifc_timer_expire+0x706/0xa30 net/ipv4/igmp.c:808
> > call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1419
> > expire_timers+0x135/0x250 kernel/time/timer.c:1464
> > __run_timers+0x358/0x420 kernel/time/timer.c:1732
> > run_timer_softirq+0x19/0x30 kernel/time/timer.c:1745
> > __do_softirq+0x12c/0x26e kernel/softirq.c:558
> > invoke_softirq kernel/softirq.c:432 [inline]
> > __irq_exit_rcu+0x9a/0xb0 kernel/softirq.c:636
> > sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1100
> > asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:638
> > console_unlock+0x8e8/0xb30 kernel/printk/printk.c:2646
> > vprintk_emit+0x125/0x3d0 kernel/printk/printk.c:2174
> > vprintk_default+0x22/0x30 kernel/printk/printk.c:2185
> > vprintk+0x15a/0x170 kernel/printk/printk_safe.c:392
> > printk+0x62/0x87 kernel/printk/printk.c:2216
> > selinux_netlink_send+0x399/0x400 security/selinux/hooks.c:6041
> > security_netlink_send+0x42/0x90 security/security.c:2070
> > netlink_sendmsg+0x59e/0x7c0 net/netlink/af_netlink.c:1919
> > sock_sendmsg_nosec net/socket.c:703 [inline]
> > sock_sendmsg net/socket.c:723 [inline]
> > ____sys_sendmsg+0x360/0x4d0 net/socket.c:2392
> > ___sys_sendmsg net/socket.c:2446 [inline]
> > __sys_sendmsg+0x1ed/0x270 net/socket.c:2475
> > __do_sys_sendmsg net/socket.c:2484 [inline]
> > __se_sys_sendmsg net/socket.c:2482 [inline]
> > __x64_sys_sendmsg+0x42/0x50 net/socket.c:2482
> > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> > entry_SYSCALL_64_after_hwframe+0x44/0xae
> >
> > value changed: 0x01 -> 0x02
> >
> > Reported by Kernel Concurrency Sanitizer on:
> > CPU: 1 PID: 12539 Comm: syz-executor.1 Not tainted 5.14.0-rc4-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: syzbot <syzkaller@googlegroups.com>
> > ---
> > net/ipv4/igmp.c | 21 ++++++++++++++-------
> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> > index 6b3c558a4f232652b97a078d48f302864e60a866..a51360087b19845a28408c827032e08dabf99838 100644
> > --- a/net/ipv4/igmp.c
> > +++ b/net/ipv4/igmp.c
> > @@ -803,10 +803,17 @@ static void igmp_gq_timer_expire(struct timer_list *t)
> > static void igmp_ifc_timer_expire(struct timer_list *t)
> > {
> > struct in_device *in_dev = from_timer(in_dev, t, mr_ifc_timer);
> > + u8 mr_ifc_count;
> >
> > igmpv3_send_cr(in_dev);
> > - if (in_dev->mr_ifc_count) {
> > - in_dev->mr_ifc_count--;
> > +restart:
> > + mr_ifc_count = READ_ONCE(in_dev->mr_ifc_count);
> > +
> > + if (mr_ifc_count) {
> > + if (cmpxchg(&in_dev->mr_ifc_count,
>
> Unfortunately riscv only supports 4-byte and 8-byte cmpxchg(),
> so this results in build errors.
>
> Building riscv:defconfig ... failed
> --------------
> Error log:
> In file included from <command-line>:
> net/ipv4/igmp.c: In function 'igmp_ifc_timer_expire':
> include/linux/compiler_types.h:328:45: error: call to '__compiletime_assert_547' declared with attribute error: BUILD_BUG failed
>
Also:
Building arm:allmodconfig ... failed
--------------
Error log:
arm-linux-gnueabi-ld: net/ipv4/igmp.o: in function `igmp_ifc_timer_expire':
igmp.c:(.text+0x9b44): undefined reference to `__bad_cmpxchg'
make[1]: *** [Makefile:1176: vmlinux] Error 1
make: *** [Makefile:220: __sub-make] Error 2
Building xtensa:allmodconfig ... failed
--------------
Error log:
xtensa-linux-ld: net/ipv4/igmp.o: in function `igmp_gq_timer_expire':
igmp.c:(.text+0x4d4c): undefined reference to `__cmpxchg_called_with_bad_pointer'
xtensa-linux-ld: net/ipv4/igmp.o: in function `igmp_ifc_timer_expire':
igmp.c:(.text+0x4dad): undefined reference to `__cmpxchg_called_with_bad_pointer'
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire()
2021-08-11 18:32 ` Guenter Roeck
@ 2021-08-11 19:45 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2021-08-11 19:45 UTC (permalink / raw)
To: Guenter Roeck
Cc: Eric Dumazet, David S . Miller, Jakub Kicinski, netdev, syzbot
Time to make mr_ifc_count a 32bit field then :/
On Wed, Aug 11, 2021 at 8:32 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Wed, Aug 11, 2021 at 11:29:25AM -0700, Guenter Roeck wrote:
> > On Tue, Aug 10, 2021 at 02:45:47AM -0700, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > Fix the data-race reported by syzbot [1]
> > > Issue here is that igmp_ifc_timer_expire() can update in_dev->mr_ifc_count
> > > while another change just occured from another context.
> > >
> > > in_dev->mr_ifc_count is only 8bit wide, so the race had little
> > > consequences.
> > >
> > > [1]
> > > BUG: KCSAN: data-race in igmp_ifc_event / igmp_ifc_timer_expire
> > >
> > > write to 0xffff8881051e3062 of 1 bytes by task 12547 on cpu 0:
> > > igmp_ifc_event+0x1d5/0x290 net/ipv4/igmp.c:821
> > > igmp_group_added+0x462/0x490 net/ipv4/igmp.c:1356
> > > ____ip_mc_inc_group+0x3ff/0x500 net/ipv4/igmp.c:1461
> > > __ip_mc_join_group+0x24d/0x2c0 net/ipv4/igmp.c:2199
> > > ip_mc_join_group_ssm+0x20/0x30 net/ipv4/igmp.c:2218
> > > do_ip_setsockopt net/ipv4/ip_sockglue.c:1285 [inline]
> > > ip_setsockopt+0x1827/0x2a80 net/ipv4/ip_sockglue.c:1423
> > > tcp_setsockopt+0x8c/0xa0 net/ipv4/tcp.c:3657
> > > sock_common_setsockopt+0x5d/0x70 net/core/sock.c:3362
> > > __sys_setsockopt+0x18f/0x200 net/socket.c:2159
> > > __do_sys_setsockopt net/socket.c:2170 [inline]
> > > __se_sys_setsockopt net/socket.c:2167 [inline]
> > > __x64_sys_setsockopt+0x62/0x70 net/socket.c:2167
> > > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > > do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> > > entry_SYSCALL_64_after_hwframe+0x44/0xae
> > >
> > > read to 0xffff8881051e3062 of 1 bytes by interrupt on cpu 1:
> > > igmp_ifc_timer_expire+0x706/0xa30 net/ipv4/igmp.c:808
> > > call_timer_fn+0x2e/0x1d0 kernel/time/timer.c:1419
> > > expire_timers+0x135/0x250 kernel/time/timer.c:1464
> > > __run_timers+0x358/0x420 kernel/time/timer.c:1732
> > > run_timer_softirq+0x19/0x30 kernel/time/timer.c:1745
> > > __do_softirq+0x12c/0x26e kernel/softirq.c:558
> > > invoke_softirq kernel/softirq.c:432 [inline]
> > > __irq_exit_rcu+0x9a/0xb0 kernel/softirq.c:636
> > > sysvec_apic_timer_interrupt+0x69/0x80 arch/x86/kernel/apic/apic.c:1100
> > > asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:638
> > > console_unlock+0x8e8/0xb30 kernel/printk/printk.c:2646
> > > vprintk_emit+0x125/0x3d0 kernel/printk/printk.c:2174
> > > vprintk_default+0x22/0x30 kernel/printk/printk.c:2185
> > > vprintk+0x15a/0x170 kernel/printk/printk_safe.c:392
> > > printk+0x62/0x87 kernel/printk/printk.c:2216
> > > selinux_netlink_send+0x399/0x400 security/selinux/hooks.c:6041
> > > security_netlink_send+0x42/0x90 security/security.c:2070
> > > netlink_sendmsg+0x59e/0x7c0 net/netlink/af_netlink.c:1919
> > > sock_sendmsg_nosec net/socket.c:703 [inline]
> > > sock_sendmsg net/socket.c:723 [inline]
> > > ____sys_sendmsg+0x360/0x4d0 net/socket.c:2392
> > > ___sys_sendmsg net/socket.c:2446 [inline]
> > > __sys_sendmsg+0x1ed/0x270 net/socket.c:2475
> > > __do_sys_sendmsg net/socket.c:2484 [inline]
> > > __se_sys_sendmsg net/socket.c:2482 [inline]
> > > __x64_sys_sendmsg+0x42/0x50 net/socket.c:2482
> > > do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> > > do_syscall_64+0x3d/0x90 arch/x86/entry/common.c:80
> > > entry_SYSCALL_64_after_hwframe+0x44/0xae
> > >
> > > value changed: 0x01 -> 0x02
> > >
> > > Reported by Kernel Concurrency Sanitizer on:
> > > CPU: 1 PID: 12539 Comm: syz-executor.1 Not tainted 5.14.0-rc4-syzkaller #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > >
> > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Reported-by: syzbot <syzkaller@googlegroups.com>
> > > ---
> > > net/ipv4/igmp.c | 21 ++++++++++++++-------
> > > 1 file changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> > > index 6b3c558a4f232652b97a078d48f302864e60a866..a51360087b19845a28408c827032e08dabf99838 100644
> > > --- a/net/ipv4/igmp.c
> > > +++ b/net/ipv4/igmp.c
> > > @@ -803,10 +803,17 @@ static void igmp_gq_timer_expire(struct timer_list *t)
> > > static void igmp_ifc_timer_expire(struct timer_list *t)
> > > {
> > > struct in_device *in_dev = from_timer(in_dev, t, mr_ifc_timer);
> > > + u8 mr_ifc_count;
> > >
> > > igmpv3_send_cr(in_dev);
> > > - if (in_dev->mr_ifc_count) {
> > > - in_dev->mr_ifc_count--;
> > > +restart:
> > > + mr_ifc_count = READ_ONCE(in_dev->mr_ifc_count);
> > > +
> > > + if (mr_ifc_count) {
> > > + if (cmpxchg(&in_dev->mr_ifc_count,
> >
> > Unfortunately riscv only supports 4-byte and 8-byte cmpxchg(),
> > so this results in build errors.
> >
> > Building riscv:defconfig ... failed
> > --------------
> > Error log:
> > In file included from <command-line>:
> > net/ipv4/igmp.c: In function 'igmp_ifc_timer_expire':
> > include/linux/compiler_types.h:328:45: error: call to '__compiletime_assert_547' declared with attribute error: BUILD_BUG failed
> >
>
> Also:
>
> Building arm:allmodconfig ... failed
> --------------
> Error log:
> arm-linux-gnueabi-ld: net/ipv4/igmp.o: in function `igmp_ifc_timer_expire':
> igmp.c:(.text+0x9b44): undefined reference to `__bad_cmpxchg'
> make[1]: *** [Makefile:1176: vmlinux] Error 1
> make: *** [Makefile:220: __sub-make] Error 2
>
> Building xtensa:allmodconfig ... failed
> --------------
> Error log:
> xtensa-linux-ld: net/ipv4/igmp.o: in function `igmp_gq_timer_expire':
> igmp.c:(.text+0x4d4c): undefined reference to `__cmpxchg_called_with_bad_pointer'
> xtensa-linux-ld: net/ipv4/igmp.o: in function `igmp_ifc_timer_expire':
> igmp.c:(.text+0x4dad): undefined reference to `__cmpxchg_called_with_bad_pointer'
>
> Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-11 19:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 9:45 [PATCH net] net: igmp: fix data-race in igmp_ifc_timer_expire() Eric Dumazet
2021-08-10 11:00 ` patchwork-bot+netdevbpf
2021-08-11 18:29 ` Guenter Roeck
2021-08-11 18:32 ` Guenter Roeck
2021-08-11 19:45 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).