[syzkaller] memory leak in tcp_md5_do_add #174
Comments
|
Christoph didn't see this one with the export branch → Done |
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
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
The text was updated successfully, but these errors were encountered: