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

[syzkaller] memory leak in tcp_md5_do_add #174

Closed
cpaasch opened this issue Mar 15, 2021 · 2 comments
Closed

[syzkaller] memory leak in tcp_md5_do_add #174

cpaasch opened this issue Mar 15, 2021 · 2 comments

Comments

@cpaasch
Copy link
Member

cpaasch commented Mar 15, 2021

sendmsg$NLBL_MGMT_C_ADDDEF(r1, &(0x7f0000000180)={0x0, 0x0, &(0x7f0000000140)={&(0x7f0000000240)=ANY=[@ANYBLOB="14000000", @ANYRES16=0x0, @ANYBLOB="009375253b32343b5d1a0408000078644760e453b4c883b8dd204d372e6514c62b0e277df8295dda6f0827c36258f3cd65d3ca0ad6790cc651233a01000000093e3019219bbd33315cf7dc8ffc8e58e02cb2d0aa94910ddcdd39de48317adabe9509325cad3dcc0981cc8ed5a2de606227a16c6200"/127], 0x14}}, 0x88c1)
BUG: memory leak
unreferenced object 0xffff888108ea8fc0 (size 32):
  comm "syz-executor.0", pid 14707, jiffies 4295891566 (age 24.585s)
  hex dump (first 32 bytes):
    00 08 6a 23 80 88 ff ff 76 32 00 00 7f 00 60 aa  ..j#....v2....`.
    c0 83 ea 08 81 88 ff ff b0 90 7b 81 ff ff ff ff  ..........{.....
  backtrace:
    [<000000004bb91791>] kmalloc include/linux/slab.h:554 [inline]
    [<000000004bb91791>] tcp_md5_do_add+0x375/0x530 net/ipv4/tcp_ipv4.c:1163
    [<000000001c9ae5bc>] tcp_v4_parse_md5_keys+0x1ef/0x440 net/ipv4/tcp_ipv4.c:1275
    [<00000000c643c9d8>] do_tcp_setsockopt net/ipv4/tcp.c:3554 [inline]
    [<00000000c643c9d8>] tcp_setsockopt+0x845/0x25f0 net/ipv4/tcp.c:3638
    [<0000000025c26205>] mptcp_setsockopt+0x51d/0x650 net/mptcp/protocol.c:2901
    [<000000009614680e>] __sys_setsockopt+0x14f/0x360 net/socket.c:2115
    [<00000000ced56819>] __do_sys_setsockopt net/socket.c:2126 [inline]
    [<00000000ced56819>] __se_sys_setsockopt net/socket.c:2123 [inline]
    [<00000000ced56819>] __x64_sys_setsockopt+0xb9/0x150 net/socket.c:2123
    [<00000000f5d8be7e>] do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
    [<00000000a012b8c7>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: memory leak
unreferenced object 0xffff8880236a0800 (size 192):
  comm "syz-executor.0", pid 14707, jiffies 4295891566 (age 24.585s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 c0 8f ea 08 81 88 ff ff  ................
    20 02 20 00 ac 14 14 bb 00 00 00 00 00 00 00 00   . .............
  backtrace:
    [<00000000ec34ad94>] kmalloc include/linux/slab.h:559 [inline]
    [<00000000ec34ad94>] sock_kmalloc net/core/sock.c:2263 [inline]
    [<00000000ec34ad94>] sock_kmalloc+0xb5/0x100 net/core/sock.c:2254
    [<000000007ada111b>] tcp_md5_do_add+0x107/0x530 net/ipv4/tcp_ipv4.c:1172
    [<000000001c9ae5bc>] tcp_v4_parse_md5_keys+0x1ef/0x440 net/ipv4/tcp_ipv4.c:1275
    [<00000000c643c9d8>] do_tcp_setsockopt net/ipv4/tcp.c:3554 [inline]
    [<00000000c643c9d8>] tcp_setsockopt+0x845/0x25f0 net/ipv4/tcp.c:3638
    [<0000000025c26205>] mptcp_setsockopt+0x51d/0x650 net/mptcp/protocol.c:2901
    [<000000009614680e>] __sys_setsockopt+0x14f/0x360 net/socket.c:2115
    [<00000000ced56819>] __do_sys_setsockopt net/socket.c:2126 [inline]
    [<00000000ced56819>] __se_sys_setsockopt net/socket.c:2123 [inline]
    [<00000000ced56819>] __x64_sys_setsockopt+0xb9/0x150 net/socket.c:2123
    [<00000000f5d8be7e>] do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
    [<00000000a012b8c7>] entry_SYSCALL_64_after_hwframe+0x44/0xae

BUG: leak checking failed

HEAD:
4dc9bac ("DO-NOT-MERGE: mptcp: enabled by default") (HEAD, tag: export/20210226T185626, mptcp_net-next/export) (2 weeks ago)
271fd8b ("DO-NOT-MERGE: mptcp: add GitHub Actions") (2 weeks ago)
085c608 ("DO-NOT-MERGE: mptcp: use kmalloc on kasan build") (2 weeks ago)
a36b8de ("selftests: mptcp: dump more info on mpjoin errors") (2 weeks ago)
41ac43d ("selftests: mptcp: init nstat history") (2 weeks ago)
6a7d555 ("selftests: mptcp: launch mptcp_connect with timeout") (2 weeks ago)
9325d7e ("mptcp: add mptcp reset option support") (2 weeks ago)
5f8de0c ("mptcp: remove unneeded check on first subflow") (2 weeks ago)
1f02e44 ("mptcp: add active MPC mibs") (2 weeks ago)
c43bff3 ("mptcp: add mib for token creation fallback") (2 weeks ago)
2d103a9 ("selftests: mptcp: signal addresses testcases") (2 weeks ago)
a146d88 ("mptcp: move to next addr when subflow creation fail") (2 weeks ago)
7ff83ce ("mptcp: export lookup_anno_list_by_saddr") (2 weeks ago)
c885ea3 ("selftests: mptcp: add testcases for removing addrs") (2 weeks ago)
318e055 ("selftests: mptcp: set addr id for removing testcases") (2 weeks ago)
524fba0 ("selftests: mptcp: add invert argument for chk_rm_nr") (2 weeks ago)
e139886 ("mptcp: remove a list of addrs when flushing") (2 weeks ago)
edb9620 ("mptcp: remove multi addresses and subflows in PM") (2 weeks ago)
aca6f0d ("mptcp: remove multi subflows in PM") (2 weeks ago)
71a9f0c ("mptcp: remove multi addresses in PM") (2 weeks ago)
daf1fc1 ("mptcp: add rm_list_rx in mptcp_pm_data") (2 weeks ago)
e6c556c ("mptcp: add rm_list in mptcp_options_received") (2 weeks ago)
64ef994 ("mptcp: add rm_list_tx in mptcp_pm_data") (2 weeks ago)
8d960b0 ("mptcp: add rm_list in mptcp_out_options") (2 weeks ago)
a398ebc ("selftests: mptcp: timeout testcases for multi addresses") (2 weeks ago)
ce3735d ("selftests: mptcp: add cfg_do_w for cfg_remove") (2 weeks ago)
d139745 ("mptcp: move to next addr when timeout") (2 weeks ago)
bf4629e ("mptcp: drop unused subflow in mptcp_pm_subflow_established") (2 weeks ago)
56e9734 ("mptcp: skip connecting the connected address") (2 weeks ago)
f3dd90f ("mptcp: drop argument port from mptcp_pm_announce_addr") (2 weeks ago)
507edaf ("mptcp: free resources when the port number is mismatched") (2 weeks ago)
1884902 ("mptcp: fix missing wakeup") (2 weeks ago)
c530cf9 ("mptcp: reset 'first' and ack_hint on subflow close") (2 weeks ago)
faa6e7b ("mptcp: fix DATA_FIN generation on early shutdown") (2 weeks ago)
bc332c8 ("mptcp: clean-up the rtx path") (2 weeks ago)
744fcac ("mptcp: fix race in release_cb") (2 weeks ago)
888b673 ("mptcp: factor out __mptcp_retrans helper()") (2 weeks ago)
9b17fa5 ("mptcp: dispose initial struct socket when its subflow is closed") (2 weeks ago)
d93f13f ("bpf:selftests: add bpf_mptcp_sock() verifier tests") (2 weeks ago)
fd93653 ("bpf:selftests: add MPTCP test base") (2 weeks ago)
6b42214 ("bpf: add 'bpf_mptcp_sock' structure and helper") (2 weeks ago)
e3071ba ("bpf: expose is_mptcp flag to bpf_tcp_sock") (2 weeks ago)
3086346 ("linux: handle MPTCP consistently with TCP") (2 weeks ago)
6e6c2ef ("mptcp: fix memory accounting on allocation error") (2 weeks ago)
b5b1a24 ("mptcp: do not wakeup listener for MPJ subflows") (2 weeks ago)
6449532 ("mptcp: put subflow sock on connect error") (2 weeks ago)
f85f772 ("mptcp: fix DATA_FIN processing for orphaned sockets") (2 weeks ago)
32fcc88 ("mptcp: provide subflow aware release function") (2 weeks ago)
fc43ad8 ("mptcp: reset last_snd on subflow close") (2 weeks ago)
d310ec0 ("Merge tag 'perf-core-2021-02-17' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip") (mptcp_net-next/net-next) (3 weeks ago)

CONFIG-file:
CONFIG.txt

@matttbe
Copy link
Member

matttbe commented Mar 15, 2021

Hi Christoph,

May you retry with the latest export branch please? This is probably fixed by the new sockopt white/allowed list added by Paolo to fix #170
→ - ce35fc3: mptcp: only admit explicitly supported sockopt

We should not accept MD5 sockopt.

@matttbe
Copy link
Member

matttbe commented Apr 1, 2021

Christoph didn't see this one with the export branch → Done

@matttbe matttbe closed this as completed Apr 1, 2021
jenkins-tessares pushed a commit that referenced this issue Apr 29, 2021
Fix BPF_CORE_READ_BITFIELD() macro used for reading CO-RE-relocatable
bitfields. Missing breaks in a switch caused 8-byte reads always. This can
confuse libbpf because it does strict checks that memory load size corresponds
to the original size of the field, which in this case quite often would be
wrong.

After fixing that, we run into another problem, which quite subtle, so worth
documenting here. The issue is in Clang optimization and CO-RE relocation
interactions. Without that asm volatile construct (also known as
barrier_var()), Clang will re-order BYTE_OFFSET and BYTE_SIZE relocations and
will apply BYTE_OFFSET 4 times for each switch case arm. This will result in
the same error from libbpf about mismatch of memory load size and original
field size. I.e., if we were reading u32, we'd still have *(u8 *), *(u16 *),
*(u32 *), and *(u64 *) memory loads, three of which will fail. Using
barrier_var() forces Clang to apply BYTE_OFFSET relocation first (and once) to
calculate p, after which value of p is used without relocation in each of
switch case arms, doing appropiately-sized memory load.

Here's the list of relevant relocations and pieces of generated BPF code
before and after this patch for test_core_reloc_bitfields_direct selftests.

BEFORE
=====
 #45: core_reloc: insn #160 --> [5] + 0:5: byte_sz --> struct core_reloc_bitfields.u32
 #46: core_reloc: insn #167 --> [5] + 0:5: byte_off --> struct core_reloc_bitfields.u32
 #47: core_reloc: insn #174 --> [5] + 0:5: byte_off --> struct core_reloc_bitfields.u32
 #48: core_reloc: insn #178 --> [5] + 0:5: byte_off --> struct core_reloc_bitfields.u32
 #49: core_reloc: insn #182 --> [5] + 0:5: byte_off --> struct core_reloc_bitfields.u32

     157:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
     159:       7b 12 20 01 00 00 00 00 *(u64 *)(r2 + 288) = r1
     160:       b7 02 00 00 04 00 00 00 r2 = 4
; BYTE_SIZE relocation here                 ^^^
     161:       66 02 07 00 03 00 00 00 if w2 s> 3 goto +7 <LBB0_63>
     162:       16 02 0d 00 01 00 00 00 if w2 == 1 goto +13 <LBB0_65>
     163:       16 02 01 00 02 00 00 00 if w2 == 2 goto +1 <LBB0_66>
     164:       05 00 12 00 00 00 00 00 goto +18 <LBB0_69>

0000000000000528 <LBB0_66>:
     165:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
     167:       69 11 08 00 00 00 00 00 r1 = *(u16 *)(r1 + 8)
; BYTE_OFFSET relo here w/ WRONG size        ^^^^^^^^^^^^^^^^
     168:       05 00 0e 00 00 00 00 00 goto +14 <LBB0_69>

0000000000000548 <LBB0_63>:
     169:       16 02 0a 00 04 00 00 00 if w2 == 4 goto +10 <LBB0_67>
     170:       16 02 01 00 08 00 00 00 if w2 == 8 goto +1 <LBB0_68>
     171:       05 00 0b 00 00 00 00 00 goto +11 <LBB0_69>

0000000000000560 <LBB0_68>:
     172:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
     174:       79 11 08 00 00 00 00 00 r1 = *(u64 *)(r1 + 8)
; BYTE_OFFSET relo here w/ WRONG size        ^^^^^^^^^^^^^^^^
     175:       05 00 07 00 00 00 00 00 goto +7 <LBB0_69>

0000000000000580 <LBB0_65>:
     176:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
     178:       71 11 08 00 00 00 00 00 r1 = *(u8 *)(r1 + 8)
; BYTE_OFFSET relo here w/ WRONG size        ^^^^^^^^^^^^^^^^
     179:       05 00 03 00 00 00 00 00 goto +3 <LBB0_69>

00000000000005a0 <LBB0_67>:
     180:       18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll
     182:       61 11 08 00 00 00 00 00 r1 = *(u32 *)(r1 + 8)
; BYTE_OFFSET relo here w/ RIGHT size        ^^^^^^^^^^^^^^^^

00000000000005b8 <LBB0_69>:
     183:       67 01 00 00 20 00 00 00 r1 <<= 32
     184:       b7 02 00 00 00 00 00 00 r2 = 0
     185:       16 02 02 00 00 00 00 00 if w2 == 0 goto +2 <LBB0_71>
     186:       c7 01 00 00 20 00 00 00 r1 s>>= 32
     187:       05 00 01 00 00 00 00 00 goto +1 <LBB0_72>

00000000000005e0 <LBB0_71>:
     188:       77 01 00 00 20 00 00 00 r1 >>= 32

AFTER
=====

 #30: core_reloc: insn #132 --> [5] + 0:5: byte_off --> struct core_reloc_bitfields.u32
 #31: core_reloc: insn #134 --> [5] + 0:5: byte_sz --> struct core_reloc_bitfields.u32

     129:       18 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r2 = 0 ll
     131:       7b 12 20 01 00 00 00 00 *(u64 *)(r2 + 288) = r1
     132:       b7 01 00 00 08 00 00 00 r1 = 8
; BYTE_OFFSET relo here                     ^^^
; no size check for non-memory dereferencing instructions
     133:       0f 12 00 00 00 00 00 00 r2 += r1
     134:       b7 03 00 00 04 00 00 00 r3 = 4
; BYTE_SIZE relocation here                 ^^^
     135:       66 03 05 00 03 00 00 00 if w3 s> 3 goto +5 <LBB0_63>
     136:       16 03 09 00 01 00 00 00 if w3 == 1 goto +9 <LBB0_65>
     137:       16 03 01 00 02 00 00 00 if w3 == 2 goto +1 <LBB0_66>
     138:       05 00 0a 00 00 00 00 00 goto +10 <LBB0_69>

0000000000000458 <LBB0_66>:
     139:       69 21 00 00 00 00 00 00 r1 = *(u16 *)(r2 + 0)
; NO CO-RE relocation here                   ^^^^^^^^^^^^^^^^
     140:       05 00 08 00 00 00 00 00 goto +8 <LBB0_69>

0000000000000468 <LBB0_63>:
     141:       16 03 06 00 04 00 00 00 if w3 == 4 goto +6 <LBB0_67>
     142:       16 03 01 00 08 00 00 00 if w3 == 8 goto +1 <LBB0_68>
     143:       05 00 05 00 00 00 00 00 goto +5 <LBB0_69>

0000000000000480 <LBB0_68>:
     144:       79 21 00 00 00 00 00 00 r1 = *(u64 *)(r2 + 0)
; NO CO-RE relocation here                   ^^^^^^^^^^^^^^^^
     145:       05 00 03 00 00 00 00 00 goto +3 <LBB0_69>

0000000000000490 <LBB0_65>:
     146:       71 21 00 00 00 00 00 00 r1 = *(u8 *)(r2 + 0)
; NO CO-RE relocation here                   ^^^^^^^^^^^^^^^^
     147:       05 00 01 00 00 00 00 00 goto +1 <LBB0_69>

00000000000004a0 <LBB0_67>:
     148:       61 21 00 00 00 00 00 00 r1 = *(u32 *)(r2 + 0)
; NO CO-RE relocation here                   ^^^^^^^^^^^^^^^^

00000000000004a8 <LBB0_69>:
     149:       67 01 00 00 20 00 00 00 r1 <<= 32
     150:       b7 02 00 00 00 00 00 00 r2 = 0
     151:       16 02 02 00 00 00 00 00 if w2 == 0 goto +2 <LBB0_71>
     152:       c7 01 00 00 20 00 00 00 r1 s>>= 32
     153:       05 00 01 00 00 00 00 00 goto +1 <LBB0_72>

00000000000004d0 <LBB0_71>:
     154:       77 01 00 00 20 00 00 00 r1 >>= 323

Fixes: ee26dad ("libbpf: Add support for relocatable bitfields")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Lorenz Bauer <lmb@cloudflare.com>
Link: https://lore.kernel.org/bpf/20210426192949.416837-4-andrii@kernel.org
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