Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kmemleak report with mptcp_connect #267

Closed
mjmartineau opened this issue Apr 16, 2022 · 3 comments
Closed

Kmemleak report with mptcp_connect #267

mjmartineau opened this issue Apr 16, 2022 · 3 comments
Assignees

Comments

@mjmartineau
Copy link
Member

Selftests are reporting a memory leak, reproducible with tag export/20220415T162900 and mptcp_connect.sh mmap:

unreferenced object 0xffff8881141bc000 (size 4096):
  comm "mptcp_connect", pid 909, jiffies 4295777828 (age 299.017s)
  hex dump (first 32 bytes):
    0a 00 01 01 0a 00 01 02 00 00 00 00 c3 52 2c a6  .............R,.
    02 00 07 40 00 00 00 00 00 00 00 00 00 00 00 00  ...@............
  backtrace:
    [<00000000705096e3>] sk_prot_alloc+0xa5/0x170
    [<00000000bf556549>] sk_alloc+0x31/0x3a0
    [<000000001b89eacc>] inet_create+0x182/0x630
    [<0000000002098173>] __sock_create+0x1c6/0x3d0
    [<00000000d64aa0cc>] __sys_socket+0xb6/0x160
    [<0000000079605cfd>] __x64_sys_socket+0x3c/0x50
    [<00000000f74a2274>] do_syscall_64+0x3b/0x90
    [<000000001638faf6>] entry_SYSCALL_64_after_hwframe+0x44/0xae
unreferenced object 0xffff88811441ae40 (size 32):
  comm "mptcp_connect", pid 909, jiffies 4295777828 (age 299.017s)
  hex dump (first 32 bytes):
    02 00 00 00 00 00 00 00 00 31 41 06 81 88 ff ff  .........1A.....
    78 01 00 00 03 00 00 00 10 00 00 00 00 00 00 00  x...............
  backtrace:
    [<00000000ebd70815>] selinux_sk_alloc_security+0x55/0xc0
    [<00000000c2bbad22>] security_sk_alloc+0x39/0x60
    [<00000000a1e1883a>] sk_prot_alloc+0xba/0x170
    [<00000000bf556549>] sk_alloc+0x31/0x3a0
    [<000000001b89eacc>] inet_create+0x182/0x630
    [<0000000002098173>] __sock_create+0x1c6/0x3d0
    [<00000000d64aa0cc>] __sys_socket+0xb6/0x160
    [<0000000079605cfd>] __x64_sys_socket+0x3c/0x50
    [<00000000f74a2274>] do_syscall_64+0x3b/0x90
    [<000000001638faf6>] entry_SYSCALL_64_after_hwframe+0x44/0xae
unreferenced object 0xffff888106413100 (size 64):
  comm "mptcp_connect", pid 909, jiffies 4295777829 (age 299.016s)
  hex dump (first 32 bytes):
    15 00 00 01 00 00 00 00 00 68 87 12 81 88 ff ff  .........h......
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000a68346cd>] selinux_netlbl_sock_genattr+0x62/0x1a0
    [<00000000ef3e4ccf>] selinux_netlbl_socket_post_create+0x3b/0xa0
    [<000000003be2ad4e>] selinux_socket_post_create+0x1a1/0x3d0
    [<00000000bac4838f>] security_socket_post_create+0x4b/0x70
    [<0000000021f7f5c6>] __sock_create+0x375/0x3d0
    [<00000000d64aa0cc>] __sys_socket+0xb6/0x160
    [<0000000079605cfd>] __x64_sys_socket+0x3c/0x50
    [<00000000f74a2274>] do_syscall_64+0x3b/0x90
    [<000000001638faf6>] entry_SYSCALL_64_after_hwframe+0x44/0xae
unreferenced object 0xffff888112876800 (size 16):
  comm "mptcp_connect", pid 909, jiffies 4295777829 (age 299.016s)
  hex dump (first 16 bytes):
    75 6e 63 6f 6e 66 69 6e 65 64 5f 74 00 88 ff ff  unconfined_t....
  backtrace:
    [<0000000028feef7d>] kstrdup+0x2e/0x60
    [<00000000a56d00b3>] security_netlbl_sid_to_secattr+0xfc/0x250
    [<00000000eb36ea98>] selinux_netlbl_sock_genattr+0x85/0x1a0
    [<00000000ef3e4ccf>] selinux_netlbl_socket_post_create+0x3b/0xa0
    [<000000003be2ad4e>] selinux_socket_post_create+0x1a1/0x3d0
    [<00000000bac4838f>] security_socket_post_create+0x4b/0x70
    [<0000000021f7f5c6>] __sock_create+0x375/0x3d0
    [<00000000d64aa0cc>] __sys_socket+0xb6/0x160
    [<0000000079605cfd>] __x64_sys_socket+0x3c/0x50
    [<00000000f74a2274>] do_syscall_64+0x3b/0x90
    [<000000001638faf6>] entry_SYSCALL_64_after_hwframe+0x44/0xae
unreferenced object 0xffff888111c98000 (size 4096):
  comm "softirq", pid 0, jiffies 4295777832 (age 299.038s)
  hex dump (first 32 bytes):
    0a 00 01 02 0a 00 01 01 00 00 00 00 a6 2c 52 c3  .............,R.
    02 00 07 41 00 00 00 00 00 00 00 00 00 00 00 00  ...A............
  backtrace:
    [<00000000705096e3>] sk_prot_alloc+0xa5/0x170
    [<000000005b035ded>] sk_clone_lock+0x44/0x7c0
    [<000000006050f8a6>] mptcp_sk_clone+0x74/0x340
    [<000000002d5dbc8c>] subflow_syn_recv_sock+0x4c9/0x8e0
    [<000000008158fcdb>] tcp_check_req+0x242/0xa10
    [<00000000aa154872>] tcp_v4_rcv+0x12a2/0x1a70
    [<000000005bb43f3d>] ip_protocol_deliver_rcu+0x5a/0x480
    [<00000000b3052b52>] ip_local_deliver_finish+0x12a/0x1f0
    [<00000000d59fd984>] ip_local_deliver+0x109/0x2f0
    [<00000000f757d1c3>] ip_rcv+0x2f9/0x400
    [<0000000029eca06e>] __netif_receive_skb_one_core+0x11f/0x130
    [<000000007e83040a>] process_backlog+0xa1/0x360
    [<00000000fbcd365f>] __napi_poll.constprop.0+0x5c/0x1e0
    [<00000000695ad1c1>] net_rx_action+0x489/0x540
    [<0000000001dd9836>] __do_softirq+0x1b7/0x626
    [<00000000d08b3db4>] do_softirq+0xd3/0x110
unreferenced object 0xffff888109cf7940 (size 32):
  comm "softirq", pid 0, jiffies 4295777832 (age 299.038s)
  hex dump (first 32 bytes):
    02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    01 00 00 00 00 00 00 00 10 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000ebd70815>] selinux_sk_alloc_security+0x55/0xc0
    [<00000000c2bbad22>] security_sk_alloc+0x39/0x60
    [<00000000a1e1883a>] sk_prot_alloc+0xba/0x170
    [<000000005b035ded>] sk_clone_lock+0x44/0x7c0
    [<000000006050f8a6>] mptcp_sk_clone+0x74/0x340
    [<000000002d5dbc8c>] subflow_syn_recv_sock+0x4c9/0x8e0
    [<000000008158fcdb>] tcp_check_req+0x242/0xa10
    [<00000000aa154872>] tcp_v4_rcv+0x12a2/0x1a70
    [<000000005bb43f3d>] ip_protocol_deliver_rcu+0x5a/0x480
    [<00000000b3052b52>] ip_local_deliver_finish+0x12a/0x1f0
    [<00000000d59fd984>] ip_local_deliver+0x109/0x2f0
    [<00000000f757d1c3>] ip_rcv+0x2f9/0x400
    [<0000000029eca06e>] __netif_receive_skb_one_core+0x11f/0x130
    [<000000007e83040a>] process_backlog+0xa1/0x360
    [<00000000fbcd365f>] __napi_poll.constprop.0+0x5c/0x1e0
    [<00000000695ad1c1>] net_rx_action+0x489/0x540
unreferenced object 0xffff888034b8e000 (size 4096):
  comm "softirq", pid 0, jiffies 4295780090 (age 296.782s)
  hex dump (first 32 bytes):
    7f 00 00 06 7f 00 00 06 00 00 00 00 d0 3a 54 c3  .............:T.
    0a 00 07 41 00 00 00 00 00 00 00 00 00 00 00 00  ...A............
  backtrace:
    [<00000000705096e3>] sk_prot_alloc+0xa5/0x170
    [<000000005b035ded>] sk_clone_lock+0x44/0x7c0
    [<000000006050f8a6>] mptcp_sk_clone+0x74/0x340
    [<000000002d5dbc8c>] subflow_syn_recv_sock+0x4c9/0x8e0
    [<000000008158fcdb>] tcp_check_req+0x242/0xa10
    [<00000000f8bb7f9c>] tcp_v6_rcv+0x106a/0x1780
    [<00000000713341f4>] ip6_protocol_deliver_rcu+0x19e/0xab0
    [<000000008bd7c63f>] ip6_input_finish+0x91/0x150
    [<00000000869eb44b>] ip6_input+0x9e/0x280
    [<00000000dc3ecf5c>] ipv6_rcv+0x10f/0x400
    [<00000000fb9c43e5>] __netif_receive_skb_one_core+0xd9/0x130
    [<000000007e83040a>] process_backlog+0xa1/0x360
    [<00000000fbcd365f>] __napi_poll.constprop.0+0x5c/0x1e0
    [<00000000695ad1c1>] net_rx_action+0x489/0x540
    [<0000000001dd9836>] __do_softirq+0x1b7/0x626
    [<00000000ca75217d>] run_ksoftirqd+0x32/0x50
unreferenced object 0xffff88800585bb00 (size 32):
  comm "softirq", pid 0, jiffies 4295780090 (age 296.807s)
  hex dump (first 32 bytes):
    02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    01 00 00 00 00 00 00 00 10 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000ebd70815>] selinux_sk_alloc_security+0x55/0xc0
    [<00000000c2bbad22>] security_sk_alloc+0x39/0x60
    [<00000000a1e1883a>] sk_prot_alloc+0xba/0x170
    [<000000005b035ded>] sk_clone_lock+0x44/0x7c0
    [<000000006050f8a6>] mptcp_sk_clone+0x74/0x340
    [<000000002d5dbc8c>] subflow_syn_recv_sock+0x4c9/0x8e0
    [<000000008158fcdb>] tcp_check_req+0x242/0xa10
    [<00000000f8bb7f9c>] tcp_v6_rcv+0x106a/0x1780
    [<00000000713341f4>] ip6_protocol_deliver_rcu+0x19e/0xab0
    [<000000008bd7c63f>] ip6_input_finish+0x91/0x150
    [<00000000869eb44b>] ip6_input+0x9e/0x280
    [<00000000dc3ecf5c>] ipv6_rcv+0x10f/0x400
    [<00000000fb9c43e5>] __netif_receive_skb_one_core+0xd9/0x130
    [<000000007e83040a>] process_backlog+0xa1/0x360
    [<00000000fbcd365f>] __napi_poll.constprop.0+0x5c/0x1e0
    [<00000000695ad1c1>] net_rx_action+0x489/0x540

The test itself passes.

@mjmartineau mjmartineau self-assigned this Apr 16, 2022
@mjmartineau mjmartineau changed the title Kmemleak report with 'mptcp_connect.sh mmap' Kmemleak report with mptcp_connect Apr 18, 2022
@mjmartineau
Copy link
Member Author

After some more experiments, this is most easily reproducible with the userspace_pm.sh script. After adding some more debug output, the PIDs in the kmemleak reports match with the mptcp_connect processes run by the userspace script. If the test subfunctions are commented out (so only the make_connection setup functions run, along with cleanup), there is no leak.

Still trying to narrow down the cause.

@mjmartineau
Copy link
Member Author

mjmartineau commented Apr 19, 2022

Bug is due to missing sock_put()s on the msks returned by mptcp_token_get_sock() in the userspace PM code. Patches incoming.

@matttbe
Copy link
Member

matttbe commented Apr 21, 2022

Fixed thanks to @kmaloor and @mjmartineau

New patches for t/upstream:

  • b5427dc: "squashed" patch 1/3 in "mptcp: netlink: Add MPTCP_PM_CMD_ANNOUNCE"
  • 93a0837: "squashed" patch 2/3 in "mptcp: netlink: Add MPTCP_PM_CMD_REMOVE"
  • 1cfb03d: "squashed" patch 3/3 in "mptcp: netlink: allow userspace-driven subflow establishment"
  • Results: 8001d7b..1e2cdcf (export)

@matttbe matttbe closed this as completed Apr 21, 2022
jenkins-tessares pushed a commit that referenced this issue Oct 6, 2023
Add various tests to check maximum number of supported programs
being attached:

  # ./vmtest.sh -- ./test_progs -t tc_opts
  [...]
  ./test_progs -t tc_opts
  [    1.185325] bpf_testmod: loading out-of-tree module taints kernel.
  [    1.186826] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
  [    1.270123] tsc: Refined TSC clocksource calibration: 3407.988 MHz
  [    1.272428] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x311fc932722, max_idle_ns: 440795381586 ns
  [    1.276408] clocksource: Switched to clocksource tsc
  #252     tc_opts_after:OK
  #253     tc_opts_append:OK
  #254     tc_opts_basic:OK
  #255     tc_opts_before:OK
  #256     tc_opts_chain_classic:OK
  #257     tc_opts_chain_mixed:OK
  #258     tc_opts_delete_empty:OK
  #259     tc_opts_demixed:OK
  #260     tc_opts_detach:OK
  #261     tc_opts_detach_after:OK
  #262     tc_opts_detach_before:OK
  #263     tc_opts_dev_cleanup:OK
  #264     tc_opts_invalid:OK
  #265     tc_opts_max:OK              <--- (new test)
  #266     tc_opts_mixed:OK
  #267     tc_opts_prepend:OK
  #268     tc_opts_replace:OK
  #269     tc_opts_revision:OK
  Summary: 18/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: Andrii Nakryiko <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
jenkins-tessares pushed a commit that referenced this issue Oct 13, 2023
Add a new test case which performs double query of the bpf_mprog through
libbpf API, but also via raw bpf(2) syscall. This is testing to gather
first the count and then in a subsequent probe the full information with
the program array without clearing passed structs in between.

  # ./vmtest.sh -- ./test_progs -t tc_opts
  [...]
  ./test_progs -t tc_opts
  [    1.398818] tsc: Refined TSC clocksource calibration: 3407.999 MHz
  [    1.400263] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x311fd336761, max_idle_ns: 440795243819 ns
  [    1.402734] clocksource: Switched to clocksource tsc
  [    1.426639] bpf_testmod: loading out-of-tree module taints kernel.
  [    1.428112] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
  #252     tc_opts_after:OK
  #253     tc_opts_append:OK
  #254     tc_opts_basic:OK
  #255     tc_opts_before:OK
  #256     tc_opts_chain_classic:OK
  #257     tc_opts_chain_mixed:OK
  #258     tc_opts_delete_empty:OK
  #259     tc_opts_demixed:OK
  #260     tc_opts_detach:OK
  #261     tc_opts_detach_after:OK
  #262     tc_opts_detach_before:OK
  #263     tc_opts_dev_cleanup:OK
  #264     tc_opts_invalid:OK
  #265     tc_opts_max:OK
  #266     tc_opts_mixed:OK
  #267     tc_opts_prepend:OK
  #268     tc_opts_query:OK            <--- (new test)
  #269     tc_opts_replace:OK
  #270     tc_opts_revision:OK
  Summary: 19/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Martin KaFai Lau <[email protected]>
jenkins-tessares pushed a commit that referenced this issue Oct 13, 2023
Add a new test case to query on an empty bpf_mprog and pass the revision
directly into expected_revision for attachment to assert that this does
succeed.

  ./test_progs -t tc_opts
  [    1.406778] tsc: Refined TSC clocksource calibration: 3407.990 MHz
  [    1.408863] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x311fcaf6eb0, max_idle_ns: 440795321766 ns
  [    1.412419] clocksource: Switched to clocksource tsc
  [    1.428671] bpf_testmod: loading out-of-tree module taints kernel.
  [    1.430260] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel
  #252     tc_opts_after:OK
  #253     tc_opts_append:OK
  #254     tc_opts_basic:OK
  #255     tc_opts_before:OK
  #256     tc_opts_chain_classic:OK
  #257     tc_opts_chain_mixed:OK
  #258     tc_opts_delete_empty:OK
  #259     tc_opts_demixed:OK
  #260     tc_opts_detach:OK
  #261     tc_opts_detach_after:OK
  #262     tc_opts_detach_before:OK
  #263     tc_opts_dev_cleanup:OK
  #264     tc_opts_invalid:OK
  #265     tc_opts_max:OK
  #266     tc_opts_mixed:OK
  #267     tc_opts_prepend:OK
  #268     tc_opts_query:OK
  #269     tc_opts_query_attach:OK     <--- (new test)
  #270     tc_opts_replace:OK
  #271     tc_opts_revision:OK
  Summary: 20/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Martin KaFai Lau <[email protected]>
matttbe pushed a commit that referenced this issue Oct 27, 2023
Add several new test cases which assert corner cases on the mprog query
mechanism, for example, around passing in a too small or a larger array
than the current count.

  ./test_progs -t tc_opts
  #252     tc_opts_after:OK
  #253     tc_opts_append:OK
  #254     tc_opts_basic:OK
  #255     tc_opts_before:OK
  #256     tc_opts_chain_classic:OK
  #257     tc_opts_chain_mixed:OK
  #258     tc_opts_delete_empty:OK
  #259     tc_opts_demixed:OK
  #260     tc_opts_detach:OK
  #261     tc_opts_detach_after:OK
  #262     tc_opts_detach_before:OK
  #263     tc_opts_dev_cleanup:OK
  #264     tc_opts_invalid:OK
  #265     tc_opts_max:OK
  #266     tc_opts_mixed:OK
  #267     tc_opts_prepend:OK
  #268     tc_opts_query:OK
  #269     tc_opts_query_attach:OK
  #270     tc_opts_replace:OK
  #271     tc_opts_revision:OK
  Summary: 20/0 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: Andrii Nakryiko <[email protected]>
Reviewed-by: Alan Maguire <[email protected]>
Link: https://lore.kernel.org/bpf/[email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants