2024-02-28 11:58:25

by Alexander Ofitserov

[permalink] [raw]
Subject: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

The gtp_link_ops operations structure for the subsystem must be
registered after registering the gtp_net_ops pernet operations structure.

Syzkaller hit 'general protection fault in gtp_genl_dump_pdp' bug:

[ 1010.702740] gtp: GTP module unloaded
[ 1010.715877] general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
[ 1010.715888] KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
[ 1010.715895] CPU: 1 PID: 128616 Comm: a.out Not tainted 6.8.0-rc6-std-def-alt1 #1
[ 1010.715899] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-alt1 04/01/2014
[ 1010.715908] RIP: 0010:gtp_newlink+0x4d7/0x9c0 [gtp]
[ 1010.715915] Code: 80 3c 02 00 0f 85 41 04 00 00 48 8b bb d8 05 00 00 e8 ed f6 ff ff 48 89 c2 48 89 c5 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f 85 4f 04 00 00 4c 89 e2 4c 8b 6d 00 48 b8 00 00 00
[ 1010.715920] RSP: 0018:ffff888020fbf180 EFLAGS: 00010203
[ 1010.715929] RAX: dffffc0000000000 RBX: ffff88800399c000 RCX: 0000000000000000
[ 1010.715933] RDX: 0000000000000001 RSI: ffffffff84805280 RDI: 0000000000000282
[ 1010.715938] RBP: 000000000000000d R08: 0000000000000001 R09: 0000000000000000
[ 1010.715942] R10: 0000000000000001 R11: 0000000000000001 R12: ffff88800399cc80
[ 1010.715947] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000400
[ 1010.715953] FS: 00007fd1509ab5c0(0000) GS:ffff88805b300000(0000) knlGS:0000000000000000
[ 1010.715958] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1010.715962] CR2: 0000000000000000 CR3: 000000001c07a000 CR4: 0000000000750ee0
[ 1010.715968] PKRU: 55555554
[ 1010.715972] Call Trace:
[ 1010.715985] ? __die_body.cold+0x1a/0x1f
[ 1010.715995] ? die_addr+0x43/0x70
[ 1010.716002] ? exc_general_protection+0x199/0x2f0
[ 1010.716016] ? asm_exc_general_protection+0x1e/0x30
[ 1010.716026] ? gtp_newlink+0x4d7/0x9c0 [gtp]
[ 1010.716034] ? gtp_net_exit+0x150/0x150 [gtp]
[ 1010.716042] __rtnl_newlink+0x1063/0x1700
[ 1010.716051] ? rtnl_setlink+0x3c0/0x3c0
[ 1010.716063] ? is_bpf_text_address+0xc0/0x1f0
[ 1010.716070] ? kernel_text_address.part.0+0xbb/0xd0
[ 1010.716076] ? __kernel_text_address+0x56/0xa0
[ 1010.716084] ? unwind_get_return_address+0x5a/0xa0
[ 1010.716091] ? create_prof_cpu_mask+0x30/0x30
[ 1010.716098] ? arch_stack_walk+0x9e/0xf0
[ 1010.716106] ? stack_trace_save+0x91/0xd0
[ 1010.716113] ? stack_trace_consume_entry+0x170/0x170
[ 1010.716121] ? __lock_acquire+0x15c5/0x5380
[ 1010.716139] ? mark_held_locks+0x9e/0xe0
[ 1010.716148] ? kmem_cache_alloc_trace+0x35f/0x3c0
[ 1010.716155] ? __rtnl_newlink+0x1700/0x1700
[ 1010.716160] rtnl_newlink+0x69/0xa0
[ 1010.716166] rtnetlink_rcv_msg+0x43b/0xc50
[ 1010.716172] ? rtnl_fdb_dump+0x9f0/0x9f0
[ 1010.716179] ? lock_acquire+0x1fe/0x560
[ 1010.716188] ? netlink_deliver_tap+0x12f/0xd50
[ 1010.716196] netlink_rcv_skb+0x14d/0x440
[ 1010.716202] ? rtnl_fdb_dump+0x9f0/0x9f0
[ 1010.716208] ? netlink_ack+0xab0/0xab0
[ 1010.716213] ? netlink_deliver_tap+0x202/0xd50
[ 1010.716220] ? netlink_deliver_tap+0x218/0xd50
[ 1010.716226] ? __virt_addr_valid+0x30b/0x590
[ 1010.716233] netlink_unicast+0x54b/0x800
[ 1010.716240] ? netlink_attachskb+0x870/0x870
[ 1010.716248] ? __check_object_size+0x2de/0x3b0
[ 1010.716254] netlink_sendmsg+0x938/0xe40
[ 1010.716261] ? netlink_unicast+0x800/0x800
[ 1010.716269] ? __import_iovec+0x292/0x510
[ 1010.716276] ? netlink_unicast+0x800/0x800
[ 1010.716284] __sock_sendmsg+0x159/0x190
[ 1010.716290] ____sys_sendmsg+0x712/0x880
[ 1010.716297] ? sock_write_iter+0x3d0/0x3d0
[ 1010.716304] ? __ia32_sys_recvmmsg+0x270/0x270
[ 1010.716309] ? lock_acquire+0x1fe/0x560
[ 1010.716315] ? drain_array_locked+0x90/0x90
[ 1010.716324] ___sys_sendmsg+0xf8/0x170
[ 1010.716331] ? sendmsg_copy_msghdr+0x170/0x170
[ 1010.716337] ? lockdep_init_map_type+0x2c7/0x860
[ 1010.716343] ? lockdep_hardirqs_on_prepare+0x430/0x430
[ 1010.716350] ? debug_mutex_init+0x33/0x70
[ 1010.716360] ? percpu_counter_add_batch+0x8b/0x140
[ 1010.716367] ? lock_acquire+0x1fe/0x560
[ 1010.716373] ? find_held_lock+0x2c/0x110
[ 1010.716384] ? __fd_install+0x1b6/0x6f0
[ 1010.716389] ? lock_downgrade+0x810/0x810
[ 1010.716396] ? __fget_light+0x222/0x290
[ 1010.716403] __sys_sendmsg+0xea/0x1b0
[ 1010.716409] ? __sys_sendmsg_sock+0x40/0x40
[ 1010.716419] ? lockdep_hardirqs_on_prepare+0x2b3/0x430
[ 1010.716425] ? syscall_enter_from_user_mode+0x1d/0x60
[ 1010.716432] do_syscall_64+0x30/0x40
[ 1010.716438] entry_SYSCALL_64_after_hwframe+0x62/0xc7
[ 1010.716444] RIP: 0033:0x7fd1508cbd49
[ 1010.716452] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ef 70 0d 00 f7 d8 64 89 01 48
[ 1010.716456] RSP: 002b:00007fff18872348 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
[ 1010.716463] RAX: ffffffffffffffda RBX: 000055f72bf0eac0 RCX: 00007fd1508cbd49
[ 1010.716468] RDX: 0000000000000000 RSI: 0000000020000280 RDI: 0000000000000006
[ 1010.716473] RBP: 00007fff18872360 R08: 00007fff18872360 R09: 00007fff18872360
[ 1010.716478] R10: 00007fff18872360 R11: 0000000000000202 R12: 000055f72bf0e1b0
[ 1010.716482] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1010.716491] Modules linked in: gtp(+) udp_tunnel ib_core uinput af_packet rfkill qrtr joydev hid_generic usbhid hid kvm_intel iTCO_wdt intel_pmc_bxt iTCO_vendor_support kvm snd_hda_codec_generic ledtrig_audio irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel snd_hda_intel nls_utf8 snd_intel_dspcfg nls_cp866 psmouse aesni_intel vfat crypto_simd fat cryptd glue_helper snd_hda_codec pcspkr snd_hda_core i2c_i801 snd_hwdep i2c_smbus xhci_pci snd_pcm lpc_ich xhci_pci_renesas xhci_hcd qemu_fw_cfg tiny_power_button button sch_fq_codel vboxvideo drm_vram_helper drm_ttm_helper ttm vboxsf vboxguest snd_seq_midi snd_seq_midi_event snd_seq snd_rawmidi snd_seq_device snd_timer snd soundcore msr fuse efi_pstore dm_mod ip_tables x_tables autofs4 virtio_gpu virtio_dma_buf drm_kms_helper cec rc_core drm virtio_rng virtio_scsi rng_core virtio_balloon virtio_blk virtio_net virtio_console net_failover failover ahci libahci libata evdev scsi_mod input_leds serio_raw virtio_pci intel_agp
[ 1010.716674] virtio_ring intel_gtt virtio [last unloaded: gtp]
[ 1010.716693] ---[ end trace 04990a4ce61e174b ]---

Cc: [email protected]
Signed-off-by: Alexander Ofitserov <[email protected]>
Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
---
drivers/net/gtp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 2129ae42c7030..0ddec4cc84093 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1903,26 +1903,26 @@ static int __init gtp_init(void)

get_random_bytes(&gtp_h_initval, sizeof(gtp_h_initval));

- err = rtnl_link_register(&gtp_link_ops);
+ err = register_pernet_subsys(&gtp_net_ops);
if (err < 0)
goto error_out;

- err = register_pernet_subsys(&gtp_net_ops);
+ err = rtnl_link_register(&gtp_link_ops);
if (err < 0)
- goto unreg_rtnl_link;
+ goto unreg_pernet_subsys;

err = genl_register_family(&gtp_genl_family);
if (err < 0)
- goto unreg_pernet_subsys;
+ goto unreg_rtnl_link;

pr_info("GTP module loaded (pdp ctx size %zd bytes)\n",
sizeof(struct pdp_ctx));
return 0;

-unreg_pernet_subsys:
- unregister_pernet_subsys(&gtp_net_ops);
unreg_rtnl_link:
rtnl_link_unregister(&gtp_link_ops);
+unreg_pernet_subsys:
+ unregister_pernet_subsys(&gtp_net_ops);
error_out:
pr_err("error loading GTP module loaded\n");
return err;
--
2.42.1



2024-02-28 13:08:10

by Jiri Pirko

[permalink] [raw]
Subject: Re: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

Wed, Feb 28, 2024 at 12:47:03PM CET, [email protected] wrote:
>The gtp_link_ops operations structure for the subsystem must be
>registered after registering the gtp_net_ops pernet operations structure.
>
>Syzkaller hit 'general protection fault in gtp_genl_dump_pdp' bug:
>
>[ 1010.702740] gtp: GTP module unloaded
>[ 1010.715877] general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
>[ 1010.715888] KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
>[ 1010.715895] CPU: 1 PID: 128616 Comm: a.out Not tainted 6.8.0-rc6-std-def-alt1 #1
>[ 1010.715899] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-alt1 04/01/2014
>[ 1010.715908] RIP: 0010:gtp_newlink+0x4d7/0x9c0 [gtp]
>[ 1010.715915] Code: 80 3c 02 00 0f 85 41 04 00 00 48 8b bb d8 05 00 00 e8 ed f6 ff ff 48 89 c2 48 89 c5 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f 85 4f 04 00 00 4c 89 e2 4c 8b 6d 00 48 b8 00 00 00
>[ 1010.715920] RSP: 0018:ffff888020fbf180 EFLAGS: 00010203
>[ 1010.715929] RAX: dffffc0000000000 RBX: ffff88800399c000 RCX: 0000000000000000
>[ 1010.715933] RDX: 0000000000000001 RSI: ffffffff84805280 RDI: 0000000000000282
>[ 1010.715938] RBP: 000000000000000d R08: 0000000000000001 R09: 0000000000000000
>[ 1010.715942] R10: 0000000000000001 R11: 0000000000000001 R12: ffff88800399cc80
>[ 1010.715947] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000400
>[ 1010.715953] FS: 00007fd1509ab5c0(0000) GS:ffff88805b300000(0000) knlGS:0000000000000000
>[ 1010.715958] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[ 1010.715962] CR2: 0000000000000000 CR3: 000000001c07a000 CR4: 0000000000750ee0
>[ 1010.715968] PKRU: 55555554
>[ 1010.715972] Call Trace:
>[ 1010.715985] ? __die_body.cold+0x1a/0x1f
>[ 1010.715995] ? die_addr+0x43/0x70
>[ 1010.716002] ? exc_general_protection+0x199/0x2f0
>[ 1010.716016] ? asm_exc_general_protection+0x1e/0x30
>[ 1010.716026] ? gtp_newlink+0x4d7/0x9c0 [gtp]
>[ 1010.716034] ? gtp_net_exit+0x150/0x150 [gtp]
>[ 1010.716042] __rtnl_newlink+0x1063/0x1700
>[ 1010.716051] ? rtnl_setlink+0x3c0/0x3c0
>[ 1010.716063] ? is_bpf_text_address+0xc0/0x1f0
>[ 1010.716070] ? kernel_text_address.part.0+0xbb/0xd0
>[ 1010.716076] ? __kernel_text_address+0x56/0xa0
>[ 1010.716084] ? unwind_get_return_address+0x5a/0xa0
>[ 1010.716091] ? create_prof_cpu_mask+0x30/0x30
>[ 1010.716098] ? arch_stack_walk+0x9e/0xf0
>[ 1010.716106] ? stack_trace_save+0x91/0xd0
>[ 1010.716113] ? stack_trace_consume_entry+0x170/0x170
>[ 1010.716121] ? __lock_acquire+0x15c5/0x5380
>[ 1010.716139] ? mark_held_locks+0x9e/0xe0
>[ 1010.716148] ? kmem_cache_alloc_trace+0x35f/0x3c0
>[ 1010.716155] ? __rtnl_newlink+0x1700/0x1700
>[ 1010.716160] rtnl_newlink+0x69/0xa0
>[ 1010.716166] rtnetlink_rcv_msg+0x43b/0xc50
>[ 1010.716172] ? rtnl_fdb_dump+0x9f0/0x9f0
>[ 1010.716179] ? lock_acquire+0x1fe/0x560
>[ 1010.716188] ? netlink_deliver_tap+0x12f/0xd50
>[ 1010.716196] netlink_rcv_skb+0x14d/0x440
>[ 1010.716202] ? rtnl_fdb_dump+0x9f0/0x9f0
>[ 1010.716208] ? netlink_ack+0xab0/0xab0
>[ 1010.716213] ? netlink_deliver_tap+0x202/0xd50
>[ 1010.716220] ? netlink_deliver_tap+0x218/0xd50
>[ 1010.716226] ? __virt_addr_valid+0x30b/0x590
>[ 1010.716233] netlink_unicast+0x54b/0x800
>[ 1010.716240] ? netlink_attachskb+0x870/0x870
>[ 1010.716248] ? __check_object_size+0x2de/0x3b0
>[ 1010.716254] netlink_sendmsg+0x938/0xe40
>[ 1010.716261] ? netlink_unicast+0x800/0x800
>[ 1010.716269] ? __import_iovec+0x292/0x510
>[ 1010.716276] ? netlink_unicast+0x800/0x800
>[ 1010.716284] __sock_sendmsg+0x159/0x190
>[ 1010.716290] ____sys_sendmsg+0x712/0x880
>[ 1010.716297] ? sock_write_iter+0x3d0/0x3d0
>[ 1010.716304] ? __ia32_sys_recvmmsg+0x270/0x270
>[ 1010.716309] ? lock_acquire+0x1fe/0x560
>[ 1010.716315] ? drain_array_locked+0x90/0x90
>[ 1010.716324] ___sys_sendmsg+0xf8/0x170
>[ 1010.716331] ? sendmsg_copy_msghdr+0x170/0x170
>[ 1010.716337] ? lockdep_init_map_type+0x2c7/0x860
>[ 1010.716343] ? lockdep_hardirqs_on_prepare+0x430/0x430
>[ 1010.716350] ? debug_mutex_init+0x33/0x70
>[ 1010.716360] ? percpu_counter_add_batch+0x8b/0x140
>[ 1010.716367] ? lock_acquire+0x1fe/0x560
>[ 1010.716373] ? find_held_lock+0x2c/0x110
>[ 1010.716384] ? __fd_install+0x1b6/0x6f0
>[ 1010.716389] ? lock_downgrade+0x810/0x810
>[ 1010.716396] ? __fget_light+0x222/0x290
>[ 1010.716403] __sys_sendmsg+0xea/0x1b0
>[ 1010.716409] ? __sys_sendmsg_sock+0x40/0x40
>[ 1010.716419] ? lockdep_hardirqs_on_prepare+0x2b3/0x430
>[ 1010.716425] ? syscall_enter_from_user_mode+0x1d/0x60
>[ 1010.716432] do_syscall_64+0x30/0x40
>[ 1010.716438] entry_SYSCALL_64_after_hwframe+0x62/0xc7
>[ 1010.716444] RIP: 0033:0x7fd1508cbd49
>[ 1010.716452] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ef 70 0d 00 f7 d8 64 89 01 48
>[ 1010.716456] RSP: 002b:00007fff18872348 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
>[ 1010.716463] RAX: ffffffffffffffda RBX: 000055f72bf0eac0 RCX: 00007fd1508cbd49
>[ 1010.716468] RDX: 0000000000000000 RSI: 0000000020000280 RDI: 0000000000000006
>[ 1010.716473] RBP: 00007fff18872360 R08: 00007fff18872360 R09: 00007fff18872360
>[ 1010.716478] R10: 00007fff18872360 R11: 0000000000000202 R12: 000055f72bf0e1b0
>[ 1010.716482] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
>[ 1010.716491] Modules linked in: gtp(+) udp_tunnel ib_core uinput af_packet rfkill qrtr joydev hid_generic usbhid hid kvm_intel iTCO_wdt intel_pmc_bxt iTCO_vendor_support kvm snd_hda_codec_generic ledtrig_audio irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel snd_hda_intel nls_utf8 snd_intel_dspcfg nls_cp866 psmouse aesni_intel vfat crypto_simd fat cryptd glue_helper snd_hda_codec pcspkr snd_hda_core i2c_i801 snd_hwdep i2c_smbus xhci_pci snd_pcm lpc_ich xhci_pci_renesas xhci_hcd qemu_fw_cfg tiny_power_button button sch_fq_codel vboxvideo drm_vram_helper drm_ttm_helper ttm vboxsf vboxguest snd_seq_midi snd_seq_midi_event snd_seq snd_rawmidi snd_seq_device snd_timer snd soundcore msr fuse efi_pstore dm_mod ip_tables x_tables autofs4 virtio_gpu virtio_dma_buf drm_kms_helper cec rc_core drm virtio_rng virtio_scsi rng_core virtio_balloon virtio_blk virtio_net virtio_console net_failover failover ahci libahci libata evdev scsi_mod input_leds serio_raw virtio_pci intel_agp
>[ 1010.716674] virtio_ring intel_gtt virtio [last unloaded: gtp]
>[ 1010.716693] ---[ end trace 04990a4ce61e174b ]---
>
>Cc: [email protected]
>Signed-off-by: Alexander Ofitserov <[email protected]>
>Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")

Reviewed-by: Jiri Pirko <[email protected]>

2024-02-29 00:01:07

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

On Wed, Feb 28, 2024 at 02:47:03PM +0300, Alexander Ofitserov wrote:
> The gtp_link_ops operations structure for the subsystem must be
> registered after registering the gtp_net_ops pernet operations structure.

A fix for this was already applied, see:

commit 136cfaca22567a03bbb3bf53a43d8cb5748b80ec
Author: Vasiliy Kovalev <[email protected]>
Date: Wed Feb 14 19:27:33 2024 +0300

gtp: fix use-after-free and null-ptr-deref in gtp_genl_dump_pdp()

> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 2129ae42c7030..0ddec4cc84093 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -1903,26 +1903,26 @@ static int __init gtp_init(void)
>
> get_random_bytes(&gtp_h_initval, sizeof(gtp_h_initval));
>
> - err = rtnl_link_register(&gtp_link_ops);
> + err = register_pernet_subsys(&gtp_net_ops);
> if (err < 0)
> goto error_out;

BTW, I like that this calls register_pernet_subsys() before
rtnl_link_register(), where a rtnetlink request could come before
pernet is set up.

> - err = register_pernet_subsys(&gtp_net_ops);
> + err = rtnl_link_register(&gtp_link_ops);
> if (err < 0)
> - goto unreg_rtnl_link;
> + goto unreg_pernet_subsys;
>
> err = genl_register_family(&gtp_genl_family);
> if (err < 0)
> - goto unreg_pernet_subsys;
> + goto unreg_rtnl_link;
>
> pr_info("GTP module loaded (pdp ctx size %zd bytes)\n",
> sizeof(struct pdp_ctx));
> return 0;
>
> -unreg_pernet_subsys:
> - unregister_pernet_subsys(&gtp_net_ops);
> unreg_rtnl_link:
> rtnl_link_unregister(&gtp_link_ops);
> +unreg_pernet_subsys:
> + unregister_pernet_subsys(&gtp_net_ops);
> error_out:
> pr_err("error loading GTP module loaded\n");
> return err;
> --
> 2.42.1
>
>

2024-02-29 09:21:18

by Vasiliy Kovalev

[permalink] [raw]
Subject: Re: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

Hi,

29.02.2024 02:51, Pablo Neira Ayuso wrote:
> On Wed, Feb 28, 2024 at 02:47:03PM +0300, Alexander Ofitserov wrote:
>> The gtp_link_ops operations structure for the subsystem must be
>> registered after registering the gtp_net_ops pernet operations structure.
>
> A fix for this was already applied, see:
>
> commit 136cfaca22567a03bbb3bf53a43d8cb5748b80ec
> Author: Vasiliy Kovalev <[email protected]>
> Date: Wed Feb 14 19:27:33 2024 +0300
>
> gtp: fix use-after-free and null-ptr-deref in gtp_genl_dump_pdp()
>
>> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
>> index 2129ae42c7030..0ddec4cc84093 100644
>> --- a/drivers/net/gtp.c
>> +++ b/drivers/net/gtp.c
>> @@ -1903,26 +1903,26 @@ static int __init gtp_init(void)
>>
>> get_random_bytes(&gtp_h_initval, sizeof(gtp_h_initval));
>>
>> - err = rtnl_link_register(&gtp_link_ops);
>> + err = register_pernet_subsys(&gtp_net_ops);
>> if (err < 0)
>> goto error_out;
>
> BTW, I like that this calls register_pernet_subsys() before
> rtnl_link_register(), where a rtnetlink request could come before
> pernet is set up.
>
>> - err = register_pernet_subsys(&gtp_net_ops);
>> + err = rtnl_link_register(&gtp_link_ops);
>> if (err < 0)
>> - goto unreg_rtnl_link;
>> + goto unreg_pernet_subsys;
>>
>> err = genl_register_family(&gtp_genl_family);
>> if (err < 0)
>> - goto unreg_pernet_subsys;
>> + goto unreg_rtnl_link;
>>
>> pr_info("GTP module loaded (pdp ctx size %zd bytes)\n",
>> sizeof(struct pdp_ctx));
>> return 0;
>>
>> -unreg_pernet_subsys:
>> - unregister_pernet_subsys(&gtp_net_ops);
>> unreg_rtnl_link:
>> rtnl_link_unregister(&gtp_link_ops);
>> +unreg_pernet_subsys:
>> + unregister_pernet_subsys(&gtp_net_ops);
>> error_out:
>> pr_err("error loading GTP module loaded\n");
>> return err;
>> --
>> 2.42.1
>>
>>

This patch fixes another problem, but a similar one, since the sequence
is incorrect when registering subsystems.

Initially, the registration sequence in the gtp module was as follows:

1) rtnl_link_register();

2) genl_register_family();

3) register_pernet_subsys();

During debugging of the module, when starting the syzkaller reproducer,
it turned out that after genl_register_family() (2),

without waiting for register_pernet_subsys()(3), the .dumpit event is
triggered, in which the data of the unregistered pernet subsystem is
accessed.

That is, the bug was fixed by the commit

136cfaca2256 ("gtp: fix use-after-free and null-ptr-deref in
gtp_genl_dump_pdp()") [1]

and the registration sequence became as follows:

1) rtnl_link_register();

2) register_pernet_subsys();

3) genl_register_family();

However, syzkaller has discovered another problem:

after registering rtnl_link_register, the .newlink event is triggered,
in which the data of the unregistered pernet subsystem is accessed.

This problem is reproducible on current stable kernels and the latest
upstream kernel 6.8-rc6, in which the patch 136cfaca2256 [1] is applied.

Therefore, the correct sequence should be as follows:

1) register_pernet_subsys();

2) rtnl_link_register();

3) genl_register_family();

The proposed patch is developed on top of the commit changes [1], does
not conflict with it and fixes the described bug.

[1]
https://lore.kernel.org/lkml/[email protected]/T/#mb1f72c2ad57b7ea6d47333e8616beccf8bce0e23

--
Regards,
Vasiliy Kovalev

2024-02-29 13:20:40

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <[email protected]>:

On Wed, 28 Feb 2024 14:47:03 +0300 you wrote:
> The gtp_link_ops operations structure for the subsystem must be
> registered after registering the gtp_net_ops pernet operations structure.
>
> Syzkaller hit 'general protection fault in gtp_genl_dump_pdp' bug:
>
> [ 1010.702740] gtp: GTP module unloaded
> [ 1010.715877] general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
> [ 1010.715888] KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> [ 1010.715895] CPU: 1 PID: 128616 Comm: a.out Not tainted 6.8.0-rc6-std-def-alt1 #1
> [ 1010.715899] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.0-alt1 04/01/2014
> [ 1010.715908] RIP: 0010:gtp_newlink+0x4d7/0x9c0 [gtp]
> [ 1010.715915] Code: 80 3c 02 00 0f 85 41 04 00 00 48 8b bb d8 05 00 00 e8 ed f6 ff ff 48 89 c2 48 89 c5 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80> 3c 02 00 0f 85 4f 04 00 00 4c 89 e2 4c 8b 6d 00 48 b8 00 00 00
> [ 1010.715920] RSP: 0018:ffff888020fbf180 EFLAGS: 00010203
> [ 1010.715929] RAX: dffffc0000000000 RBX: ffff88800399c000 RCX: 0000000000000000
> [ 1010.715933] RDX: 0000000000000001 RSI: ffffffff84805280 RDI: 0000000000000282
> [ 1010.715938] RBP: 000000000000000d R08: 0000000000000001 R09: 0000000000000000
> [ 1010.715942] R10: 0000000000000001 R11: 0000000000000001 R12: ffff88800399cc80
> [ 1010.715947] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000400
> [ 1010.715953] FS: 00007fd1509ab5c0(0000) GS:ffff88805b300000(0000) knlGS:0000000000000000
> [ 1010.715958] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1010.715962] CR2: 0000000000000000 CR3: 000000001c07a000 CR4: 0000000000750ee0
> [ 1010.715968] PKRU: 55555554
> [ 1010.715972] Call Trace:
> [ 1010.715985] ? __die_body.cold+0x1a/0x1f
> [ 1010.715995] ? die_addr+0x43/0x70
> [ 1010.716002] ? exc_general_protection+0x199/0x2f0
> [ 1010.716016] ? asm_exc_general_protection+0x1e/0x30
> [ 1010.716026] ? gtp_newlink+0x4d7/0x9c0 [gtp]
> [ 1010.716034] ? gtp_net_exit+0x150/0x150 [gtp]
> [ 1010.716042] __rtnl_newlink+0x1063/0x1700
> [ 1010.716051] ? rtnl_setlink+0x3c0/0x3c0
> [ 1010.716063] ? is_bpf_text_address+0xc0/0x1f0
> [ 1010.716070] ? kernel_text_address.part.0+0xbb/0xd0
> [ 1010.716076] ? __kernel_text_address+0x56/0xa0
> [ 1010.716084] ? unwind_get_return_address+0x5a/0xa0
> [ 1010.716091] ? create_prof_cpu_mask+0x30/0x30
> [ 1010.716098] ? arch_stack_walk+0x9e/0xf0
> [ 1010.716106] ? stack_trace_save+0x91/0xd0
> [ 1010.716113] ? stack_trace_consume_entry+0x170/0x170
> [ 1010.716121] ? __lock_acquire+0x15c5/0x5380
> [ 1010.716139] ? mark_held_locks+0x9e/0xe0
> [ 1010.716148] ? kmem_cache_alloc_trace+0x35f/0x3c0
> [ 1010.716155] ? __rtnl_newlink+0x1700/0x1700
> [ 1010.716160] rtnl_newlink+0x69/0xa0
> [ 1010.716166] rtnetlink_rcv_msg+0x43b/0xc50
> [ 1010.716172] ? rtnl_fdb_dump+0x9f0/0x9f0
> [ 1010.716179] ? lock_acquire+0x1fe/0x560
> [ 1010.716188] ? netlink_deliver_tap+0x12f/0xd50
> [ 1010.716196] netlink_rcv_skb+0x14d/0x440
> [ 1010.716202] ? rtnl_fdb_dump+0x9f0/0x9f0
> [ 1010.716208] ? netlink_ack+0xab0/0xab0
> [ 1010.716213] ? netlink_deliver_tap+0x202/0xd50
> [ 1010.716220] ? netlink_deliver_tap+0x218/0xd50
> [ 1010.716226] ? __virt_addr_valid+0x30b/0x590
> [ 1010.716233] netlink_unicast+0x54b/0x800
> [ 1010.716240] ? netlink_attachskb+0x870/0x870
> [ 1010.716248] ? __check_object_size+0x2de/0x3b0
> [ 1010.716254] netlink_sendmsg+0x938/0xe40
> [ 1010.716261] ? netlink_unicast+0x800/0x800
> [ 1010.716269] ? __import_iovec+0x292/0x510
> [ 1010.716276] ? netlink_unicast+0x800/0x800
> [ 1010.716284] __sock_sendmsg+0x159/0x190
> [ 1010.716290] ____sys_sendmsg+0x712/0x880
> [ 1010.716297] ? sock_write_iter+0x3d0/0x3d0
> [ 1010.716304] ? __ia32_sys_recvmmsg+0x270/0x270
> [ 1010.716309] ? lock_acquire+0x1fe/0x560
> [ 1010.716315] ? drain_array_locked+0x90/0x90
> [ 1010.716324] ___sys_sendmsg+0xf8/0x170
> [ 1010.716331] ? sendmsg_copy_msghdr+0x170/0x170
> [ 1010.716337] ? lockdep_init_map_type+0x2c7/0x860
> [ 1010.716343] ? lockdep_hardirqs_on_prepare+0x430/0x430
> [ 1010.716350] ? debug_mutex_init+0x33/0x70
> [ 1010.716360] ? percpu_counter_add_batch+0x8b/0x140
> [ 1010.716367] ? lock_acquire+0x1fe/0x560
> [ 1010.716373] ? find_held_lock+0x2c/0x110
> [ 1010.716384] ? __fd_install+0x1b6/0x6f0
> [ 1010.716389] ? lock_downgrade+0x810/0x810
> [ 1010.716396] ? __fget_light+0x222/0x290
> [ 1010.716403] __sys_sendmsg+0xea/0x1b0
> [ 1010.716409] ? __sys_sendmsg_sock+0x40/0x40
> [ 1010.716419] ? lockdep_hardirqs_on_prepare+0x2b3/0x430
> [ 1010.716425] ? syscall_enter_from_user_mode+0x1d/0x60
> [ 1010.716432] do_syscall_64+0x30/0x40
> [ 1010.716438] entry_SYSCALL_64_after_hwframe+0x62/0xc7
> [ 1010.716444] RIP: 0033:0x7fd1508cbd49
> [ 1010.716452] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ef 70 0d 00 f7 d8 64 89 01 48
> [ 1010.716456] RSP: 002b:00007fff18872348 EFLAGS: 00000202 ORIG_RAX: 000000000000002e
> [ 1010.716463] RAX: ffffffffffffffda RBX: 000055f72bf0eac0 RCX: 00007fd1508cbd49
> [ 1010.716468] RDX: 0000000000000000 RSI: 0000000020000280 RDI: 0000000000000006
> [ 1010.716473] RBP: 00007fff18872360 R08: 00007fff18872360 R09: 00007fff18872360
> [ 1010.716478] R10: 00007fff18872360 R11: 0000000000000202 R12: 000055f72bf0e1b0
> [ 1010.716482] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 1010.716491] Modules linked in: gtp(+) udp_tunnel ib_core uinput af_packet rfkill qrtr joydev hid_generic usbhid hid kvm_intel iTCO_wdt intel_pmc_bxt iTCO_vendor_support kvm snd_hda_codec_generic ledtrig_audio irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel snd_hda_intel nls_utf8 snd_intel_dspcfg nls_cp866 psmouse aesni_intel vfat crypto_simd fat cryptd glue_helper snd_hda_codec pcspkr snd_hda_core i2c_i801 snd_hwdep i2c_smbus xhci_pci snd_pcm lpc_ich xhci_pci_renesas xhci_hcd qemu_fw_cfg tiny_power_button button sch_fq_codel vboxvideo drm_vram_helper drm_ttm_helper ttm vboxsf vboxguest snd_seq_midi snd_seq_midi_event snd_seq snd_rawmidi snd_seq_device snd_timer snd soundcore msr fuse efi_pstore dm_mod ip_tables x_tables autofs4 virtio_gpu virtio_dma_buf drm_kms_helper cec rc_core drm virtio_rng virtio_scsi rng_core virtio_balloon virtio_blk virtio_net virtio_console net_failover failover ahci libahci libata evdev scsi_mod input_leds serio_raw virtio_pci
intel_agp
> [ 1010.716674] virtio_ring intel_gtt virtio [last unloaded: gtp]
> [ 1010.716693] ---[ end trace 04990a4ce61e174b ]---
>
> [...]

Here is the summary with links:
- [net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()
https://git.kernel.org/netdev/net/c/616d82c3cfa2

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



2024-02-29 16:49:59

by Pablo Neira Ayuso

[permalink] [raw]
Subject: Re: [PATCH net] gtp: fix use-after-free and null-ptr-deref in gtp_newlink()

On Thu, Feb 29, 2024 at 11:37:28AM +0300, Vasiliy Kovalev wrote:
[...]
> This patch fixes another problem, but a similar one, since the sequence is
> incorrect when registering subsystems.
>
> Initially, the registration sequence in the gtp module was as follows:
>
> 1) rtnl_link_register();
>
> 2) genl_register_family();
>
> 3) register_pernet_subsys();
>
> During debugging of the module, when starting the syzkaller reproducer, it
> turned out that after genl_register_family() (2),
>
> without waiting for register_pernet_subsys()(3), the /.dumpit/  event is
> triggered, in which the data of the unregistered pernet subsystem is
> accessed.
>
> That is, the bug was fixed by the commit
>
> 136cfaca2256 ("gtp: fix use-after-free and null-ptr-deref in gtp_genl_dump_pdp()")[1]
>
> and the registration sequence became as follows:
>
> 1) rtnl_link_register();
>
> 2) register_pernet_subsys();
>
> 3) genl_register_family();
>
> However, syzkaller has discovered another problem:
>
> after registering rtnl_link_register, the .newlink event is triggered, in
> which the data of the unregistered pernet subsystem is accessed.
>
> This problem is reproducible on current stable kernels and the latest
> upstream kernel 6.8-rc6, in which the patch 136cfaca2256 [1] is applied.
>
> Therefore, the correct sequence should be as follows:
>
> 1)register_pernet_subsys();
>
> 2) rtnl_link_register();
>
> 3) genl_register_family();
>
> The proposed patch is developed on top of the commit changes [1], does not
> conflict with it and fixes the described bug.
>
> [1] https://lore.kernel.org/lkml/[email protected]/T/#mb1f72c2ad57b7ea6d47333e8616beccf8bce0e23

Thanks for explaining, fix LGTM.