Re: [PATCH net-next] bridge: fix gc_timer mod/del race condition
From: Nikolay Aleksandrov
Date: Mon, 12 Oct 2015 17:55:55 +0200
> From: Nikolay Aleksandrov
>
> commit c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
> switchdev") introduced a timer race condition because the gc_timer can
> get rearmed after it's supposedly stopped and flushed in br_dev_delete()
> leading to a use of freed memory. So take rtnl to sync with bridge
> destruction when setting ageing_timer.
> Here's the trace reproduced with these two commands running in parallel:
> while :; do echo 1 > /sys/class/net/br0/bridge/ageing_timer; done;
> while :; do brctl addbr br0; ip l set br0 up; ip l set br0 down;
> brctl delbr br0; done;
...
> Fixes: c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
> switchdev")
> Signed-off-by: Nikolay Aleksandrov
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next] bridge: fix gc_timer mod/del race condition
On Mon, Oct 12, 2015 at 8:55 AM, Nikolay Aleksandrov
wrote:
> From: Nikolay Aleksandrov
>
> commit c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
> switchdev") introduced a timer race condition because the gc_timer can
> get rearmed after it's supposedly stopped and flushed in br_dev_delete()
> leading to a use of freed memory. So take rtnl to sync with bridge
> destruction when setting ageing_timer.
> Here's the trace reproduced with these two commands running in parallel:
> while :; do echo 1 > /sys/class/net/br0/bridge/ageing_timer; done;
> while :; do brctl addbr br0; ip l set br0 up; ip l set br0 down;
> brctl delbr br0; done;
>
> [ 300.29] BUG: unable to handle kernel paging request at
> 811c59d3
> [ 300.000263] IP: [] __internal_add_timer+0x2e/0xd0
> [ 300.000422] PGD 1a0f067 PUD 1a10063 PMD 10001e1
> [ 300.000639] Oops: 0003 [#1] SMP
> [ 300.000793] Modules linked in: bridge stp llc nfsd auth_rpcgss
> oid_registry nfs_acl nfs lockd grace fscache sunrpc crct10dif_pclmul
> crc32_pclmul crc32c_intel ghash_clmulni_intel ppdev aesni_intel
> aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd
> snd_hda_codec_generic qxl drm_kms_helper psmouse pcspkr ttm
> snd_hda_intel 9pnet_virtio evdev serio_raw joydev snd_hda_codec 9pnet
> virtio_balloon drm snd_hwdep virtio_console snd_hda_core pvpanic snd_pcm
> i2c_piix4 snd_timer acpi_cpufreq parport_pc snd parport soundcore button
> processor i2c_core ipv6 autofs4 hid_generic usbhid hid ext4 crc16
> mbcache jbd2 sg sr_mod cdrom ata_generic virtio_blk virtio_net e1000
> ehci_pci uhci_hcd ehci_hcd usbcore usb_common floppy ata_piix libata
> virtio_pci virtio_ring virtio scsi_mod
> [ 300.004008] CPU: 1 PID: 1169 Comm: bash Not tainted 4.3.0-rc3+ #46
> [ 300.004008] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 300.004008] task: 880035be2200 ti: 88003795c000 task.ti:
> 88003795c000
> [ 300.004008] RIP: 0010:[] []
> __internal_add_timer+0x2e/0xd0
> [ 300.004008] RSP: 0018:88003fd03e78 EFLAGS: 00010046
> [ 300.004008] RAX: 88003fd0ef60 RBX: 840fc78949c08548 RCX:
> 0001
> [ 300.004008] RDX: RSI: 811c59d3 RDI:
> 88003fd0df00
> [ 300.004008] RBP: 88003fd03e78 R08: R09:
>
> [ 300.004008] R10: R11: R12:
> 88003fd0df00
> [ 300.004008] R13: R14: 0001 R15:
> 816032e0
> [ 300.004008] FS: 7fcbdd609700() GS:88003fd0()
> knlGS:
> [ 300.004008] CS: 0010 DS: ES: CR0: 80050033
> [ 300.004008] CR2: 811c59d3 CR3: 37879000 CR4:
> 000406e0
> [ 300.004008] Stack:
> [ 300.004008] 88003fd03ea8 810f1775 88003c8cb958
> 88003fd0df00
> [ 300.004008] 0001 88003fd03f18
> 810f28c4
> [ 300.004008] 88003fd0eb68 88003fd0e968 88003fd0e768
> 88003fd0df68
> [ 300.004008] Call Trace:
> [ 300.004008]
> [ 300.004008] [] cascade+0x45/0x70
> [ 300.004008] [] run_timer_softirq+0x2f4/0x340
> [ 300.004008] [] __do_softirq+0xd0/0x440
> [ 300.004008] [] irq_exit+0xb3/0xc0
> [ 300.004008] [] smp_apic_timer_interrupt+0x42/0x50
> [ 300.004008] [] apic_timer_interrupt+0x87/0x90
> [ 300.004008]
> [ 300.004008] [] ? create_object+0x13c/0x2e0
> [ 300.004008] [] ? __kernel_text_address+0x4e/0x70
> [ 300.004008] [] ? __kernel_text_address+0x4e/0x70
> [ 300.004008] [] print_context_stack+0x7f/0xf0
> [ 300.004008] [] dump_trace+0x11b/0x300
> [ 300.004008] [] save_stack_trace+0x2b/0x50
> [ 300.004008] [] create_object+0x13c/0x2e0
> [ 300.004008] [] kmemleak_alloc+0x4e/0xb0
> [ 300.004008] [] kmem_cache_alloc_trace+0x18d/0x2f0
> [ 300.004008] [] kernfs_fop_open+0xc9/0x380
> [ 300.004008] [] do_dentry_open+0x1ff/0x2f0
> [ 300.004008] [] ? kernfs_fop_release+0x70/0x70
> [ 300.004008] [] vfs_open+0x59/0x60
> [ 300.004008] [] path_openat+0x1ce/0x1260
> [ 300.004008] [] do_filp_open+0x7e/0xe0
> [ 300.004008] [] ? __alloc_fd+0xaf/0x180
> [ 300.004008] [] do_sys_open+0x12b/0x210
> [ 300.004008] [] SyS_open+0x1e/0x20
> [ 300.004008] [] entry_SYSCALL_64_fastpath+0x16/0x7a
> [ 300.004008] Code: 66 90 48 8b 46 10 48 8b 4f 40 55 48 89 c2 48 89 e5
> 48 29 ca 48 81 fa ff 00 00 00 77 20 0f b6 c0 48 8d 44 c7 68 48 8b 10 48
> 85 d2 <48> 89 16 74 04 48 89 72 08 48 89 30 48 89 46 08 5d c3 48 81 fa
> [ 300.004008] RIP [] __internal_add_timer+0x2e/0xd0
> [ 300.004008] RSP
> [ 300.004008] CR2: 811c59d3
>
> Fixes: c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
> switchdev")
> Signed-off-by: Nikolay Aleksandrov
> ---
> net/bridge/br_sysfs_br.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 04ef1926ee7e..8365bd53c421 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/brid
Re: [PATCH net-next] bridge: fix gc_timer mod/del race condition
Mon, Oct 12, 2015 at 05:55:55PM CEST, ra...@blackwall.org wrote:
>From: Nikolay Aleksandrov
>
>commit c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
>switchdev") introduced a timer race condition because the gc_timer can
>get rearmed after it's supposedly stopped and flushed in br_dev_delete()
>leading to a use of freed memory. So take rtnl to sync with bridge
>destruction when setting ageing_timer.
>Here's the trace reproduced with these two commands running in parallel:
>while :; do echo 1 > /sys/class/net/br0/bridge/ageing_timer; done;
>while :; do brctl addbr br0; ip l set br0 up; ip l set br0 down;
>brctl delbr br0; done;
>
>[ 300.29] BUG: unable to handle kernel paging request at
>811c59d3
>[ 300.000263] IP: [] __internal_add_timer+0x2e/0xd0
>[ 300.000422] PGD 1a0f067 PUD 1a10063 PMD 10001e1
>[ 300.000639] Oops: 0003 [#1] SMP
>[ 300.000793] Modules linked in: bridge stp llc nfsd auth_rpcgss
>oid_registry nfs_acl nfs lockd grace fscache sunrpc crct10dif_pclmul
>crc32_pclmul crc32c_intel ghash_clmulni_intel ppdev aesni_intel
>aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd
>snd_hda_codec_generic qxl drm_kms_helper psmouse pcspkr ttm
>snd_hda_intel 9pnet_virtio evdev serio_raw joydev snd_hda_codec 9pnet
>virtio_balloon drm snd_hwdep virtio_console snd_hda_core pvpanic snd_pcm
>i2c_piix4 snd_timer acpi_cpufreq parport_pc snd parport soundcore button
>processor i2c_core ipv6 autofs4 hid_generic usbhid hid ext4 crc16
>mbcache jbd2 sg sr_mod cdrom ata_generic virtio_blk virtio_net e1000
>ehci_pci uhci_hcd ehci_hcd usbcore usb_common floppy ata_piix libata
>virtio_pci virtio_ring virtio scsi_mod
>[ 300.004008] CPU: 1 PID: 1169 Comm: bash Not tainted 4.3.0-rc3+ #46
>[ 300.004008] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>[ 300.004008] task: 880035be2200 ti: 88003795c000 task.ti:
>88003795c000
>[ 300.004008] RIP: 0010:[] []
>__internal_add_timer+0x2e/0xd0
>[ 300.004008] RSP: 0018:88003fd03e78 EFLAGS: 00010046
>[ 300.004008] RAX: 88003fd0ef60 RBX: 840fc78949c08548 RCX:
>0001
>[ 300.004008] RDX: RSI: 811c59d3 RDI:
>88003fd0df00
>[ 300.004008] RBP: 88003fd03e78 R08: R09:
>
>[ 300.004008] R10: R11: R12:
>88003fd0df00
>[ 300.004008] R13: R14: 0001 R15:
>816032e0
>[ 300.004008] FS: 7fcbdd609700() GS:88003fd0()
>knlGS:
>[ 300.004008] CS: 0010 DS: ES: CR0: 80050033
>[ 300.004008] CR2: 811c59d3 CR3: 37879000 CR4:
>000406e0
>[ 300.004008] Stack:
>[ 300.004008] 88003fd03ea8 810f1775 88003c8cb958
>88003fd0df00
>[ 300.004008] 0001 88003fd03f18
>810f28c4
>[ 300.004008] 88003fd0eb68 88003fd0e968 88003fd0e768
>88003fd0df68
>[ 300.004008] Call Trace:
>[ 300.004008]
>[ 300.004008] [] cascade+0x45/0x70
>[ 300.004008] [] run_timer_softirq+0x2f4/0x340
>[ 300.004008] [] __do_softirq+0xd0/0x440
>[ 300.004008] [] irq_exit+0xb3/0xc0
>[ 300.004008] [] smp_apic_timer_interrupt+0x42/0x50
>[ 300.004008] [] apic_timer_interrupt+0x87/0x90
>[ 300.004008]
>[ 300.004008] [] ? create_object+0x13c/0x2e0
>[ 300.004008] [] ? __kernel_text_address+0x4e/0x70
>[ 300.004008] [] ? __kernel_text_address+0x4e/0x70
>[ 300.004008] [] print_context_stack+0x7f/0xf0
>[ 300.004008] [] dump_trace+0x11b/0x300
>[ 300.004008] [] save_stack_trace+0x2b/0x50
>[ 300.004008] [] create_object+0x13c/0x2e0
>[ 300.004008] [] kmemleak_alloc+0x4e/0xb0
>[ 300.004008] [] kmem_cache_alloc_trace+0x18d/0x2f0
>[ 300.004008] [] kernfs_fop_open+0xc9/0x380
>[ 300.004008] [] do_dentry_open+0x1ff/0x2f0
>[ 300.004008] [] ? kernfs_fop_release+0x70/0x70
>[ 300.004008] [] vfs_open+0x59/0x60
>[ 300.004008] [] path_openat+0x1ce/0x1260
>[ 300.004008] [] do_filp_open+0x7e/0xe0
>[ 300.004008] [] ? __alloc_fd+0xaf/0x180
>[ 300.004008] [] do_sys_open+0x12b/0x210
>[ 300.004008] [] SyS_open+0x1e/0x20
>[ 300.004008] [] entry_SYSCALL_64_fastpath+0x16/0x7a
>[ 300.004008] Code: 66 90 48 8b 46 10 48 8b 4f 40 55 48 89 c2 48 89 e5
>48 29 ca 48 81 fa ff 00 00 00 77 20 0f b6 c0 48 8d 44 c7 68 48 8b 10 48
>85 d2 <48> 89 16 74 04 48 89 72 08 48 89 30 48 89 46 08 5d c3 48 81 fa
>[ 300.004008] RIP [] __internal_add_timer+0x2e/0xd0
>[ 300.004008] RSP
>[ 300.004008] CR2: 811c59d3
>
>Fixes: c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
>switchdev")
>Signed-off-by: Nikolay Aleksandrov
Reviewed-by: Jiri Pirko
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH net-next] bridge: fix gc_timer mod/del race condition
From: Nikolay Aleksandrov
commit c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
switchdev") introduced a timer race condition because the gc_timer can
get rearmed after it's supposedly stopped and flushed in br_dev_delete()
leading to a use of freed memory. So take rtnl to sync with bridge
destruction when setting ageing_timer.
Here's the trace reproduced with these two commands running in parallel:
while :; do echo 1 > /sys/class/net/br0/bridge/ageing_timer; done;
while :; do brctl addbr br0; ip l set br0 up; ip l set br0 down;
brctl delbr br0; done;
[ 300.29] BUG: unable to handle kernel paging request at
811c59d3
[ 300.000263] IP: [] __internal_add_timer+0x2e/0xd0
[ 300.000422] PGD 1a0f067 PUD 1a10063 PMD 10001e1
[ 300.000639] Oops: 0003 [#1] SMP
[ 300.000793] Modules linked in: bridge stp llc nfsd auth_rpcgss
oid_registry nfs_acl nfs lockd grace fscache sunrpc crct10dif_pclmul
crc32_pclmul crc32c_intel ghash_clmulni_intel ppdev aesni_intel
aes_x86_64 glue_helper lrw gf128mul ablk_helper cryptd
snd_hda_codec_generic qxl drm_kms_helper psmouse pcspkr ttm
snd_hda_intel 9pnet_virtio evdev serio_raw joydev snd_hda_codec 9pnet
virtio_balloon drm snd_hwdep virtio_console snd_hda_core pvpanic snd_pcm
i2c_piix4 snd_timer acpi_cpufreq parport_pc snd parport soundcore button
processor i2c_core ipv6 autofs4 hid_generic usbhid hid ext4 crc16
mbcache jbd2 sg sr_mod cdrom ata_generic virtio_blk virtio_net e1000
ehci_pci uhci_hcd ehci_hcd usbcore usb_common floppy ata_piix libata
virtio_pci virtio_ring virtio scsi_mod
[ 300.004008] CPU: 1 PID: 1169 Comm: bash Not tainted 4.3.0-rc3+ #46
[ 300.004008] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 300.004008] task: 880035be2200 ti: 88003795c000 task.ti:
88003795c000
[ 300.004008] RIP: 0010:[] []
__internal_add_timer+0x2e/0xd0
[ 300.004008] RSP: 0018:88003fd03e78 EFLAGS: 00010046
[ 300.004008] RAX: 88003fd0ef60 RBX: 840fc78949c08548 RCX:
0001
[ 300.004008] RDX: RSI: 811c59d3 RDI:
88003fd0df00
[ 300.004008] RBP: 88003fd03e78 R08: R09:
[ 300.004008] R10: R11: R12:
88003fd0df00
[ 300.004008] R13: R14: 0001 R15:
816032e0
[ 300.004008] FS: 7fcbdd609700() GS:88003fd0()
knlGS:
[ 300.004008] CS: 0010 DS: ES: CR0: 80050033
[ 300.004008] CR2: 811c59d3 CR3: 37879000 CR4:
000406e0
[ 300.004008] Stack:
[ 300.004008] 88003fd03ea8 810f1775 88003c8cb958
88003fd0df00
[ 300.004008] 0001 88003fd03f18
810f28c4
[ 300.004008] 88003fd0eb68 88003fd0e968 88003fd0e768
88003fd0df68
[ 300.004008] Call Trace:
[ 300.004008]
[ 300.004008] [] cascade+0x45/0x70
[ 300.004008] [] run_timer_softirq+0x2f4/0x340
[ 300.004008] [] __do_softirq+0xd0/0x440
[ 300.004008] [] irq_exit+0xb3/0xc0
[ 300.004008] [] smp_apic_timer_interrupt+0x42/0x50
[ 300.004008] [] apic_timer_interrupt+0x87/0x90
[ 300.004008]
[ 300.004008] [] ? create_object+0x13c/0x2e0
[ 300.004008] [] ? __kernel_text_address+0x4e/0x70
[ 300.004008] [] ? __kernel_text_address+0x4e/0x70
[ 300.004008] [] print_context_stack+0x7f/0xf0
[ 300.004008] [] dump_trace+0x11b/0x300
[ 300.004008] [] save_stack_trace+0x2b/0x50
[ 300.004008] [] create_object+0x13c/0x2e0
[ 300.004008] [] kmemleak_alloc+0x4e/0xb0
[ 300.004008] [] kmem_cache_alloc_trace+0x18d/0x2f0
[ 300.004008] [] kernfs_fop_open+0xc9/0x380
[ 300.004008] [] do_dentry_open+0x1ff/0x2f0
[ 300.004008] [] ? kernfs_fop_release+0x70/0x70
[ 300.004008] [] vfs_open+0x59/0x60
[ 300.004008] [] path_openat+0x1ce/0x1260
[ 300.004008] [] do_filp_open+0x7e/0xe0
[ 300.004008] [] ? __alloc_fd+0xaf/0x180
[ 300.004008] [] do_sys_open+0x12b/0x210
[ 300.004008] [] SyS_open+0x1e/0x20
[ 300.004008] [] entry_SYSCALL_64_fastpath+0x16/0x7a
[ 300.004008] Code: 66 90 48 8b 46 10 48 8b 4f 40 55 48 89 c2 48 89 e5
48 29 ca 48 81 fa ff 00 00 00 77 20 0f b6 c0 48 8d 44 c7 68 48 8b 10 48
85 d2 <48> 89 16 74 04 48 89 72 08 48 89 30 48 89 46 08 5d c3 48 81 fa
[ 300.004008] RIP [] __internal_add_timer+0x2e/0xd0
[ 300.004008] RSP
[ 300.004008] CR2: 811c59d3
Fixes: c62987bbd8a1 ("bridge: push bridge setting ageing_time down to
switchdev")
Signed-off-by: Nikolay Aleksandrov
---
net/bridge/br_sysfs_br.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
index 04ef1926ee7e..8365bd53c421 100644
--- a/net/bridge/br_sysfs_br.c
+++ b/net/bridge/br_sysfs_br.c
@@ -102,7 +102,15 @@ static ssize_t ageing_time_show(struct device *d,
static int set_ageing_time(struct net_bridge *br, unsigned long val)
{
- return br_set_ageing_time(br, val);
+ int ret;
+
+ if (!rtnl_trylock())
+
