From: Greg Kroah-Hartman Date: Fri, 8 Jul 2022 11:26:33 +0000 (+0200) Subject: 5.10-stable patches X-Git-Tag: v4.9.323~53 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=65fc742a1e0bf52b01cf8419f3080051f5fa1368;p=thirdparty%2Fkernel%2Fstable-queue.git 5.10-stable patches added patches: bpf-fix-incorrect-verifier-simulation-around-jmp32-s-jeq-jne.patch bpf-fix-insufficient-bounds-propagation-from-adjust_scalar_min_max_vals.patch net-rose-fix-uaf-bug-caused-by-rose_t0timer_expiry.patch netfilter-nf_tables-stricter-validation-of-element-data.patch netfilter-nft_set_pipapo-release-elements-in-clone-from-abort-path.patch usbnet-fix-memory-leak-in-error-case.patch --- diff --git a/queue-5.10/bpf-fix-incorrect-verifier-simulation-around-jmp32-s-jeq-jne.patch b/queue-5.10/bpf-fix-incorrect-verifier-simulation-around-jmp32-s-jeq-jne.patch new file mode 100644 index 00000000000..08d575a1fb9 --- /dev/null +++ b/queue-5.10/bpf-fix-incorrect-verifier-simulation-around-jmp32-s-jeq-jne.patch @@ -0,0 +1,119 @@ +From a12ca6277eca6aeeccf66e840c23a2b520e24c8f Mon Sep 17 00:00:00 2001 +From: Daniel Borkmann +Date: Fri, 1 Jul 2022 14:47:24 +0200 +Subject: bpf: Fix incorrect verifier simulation around jmp32's jeq/jne + +From: Daniel Borkmann + +commit a12ca6277eca6aeeccf66e840c23a2b520e24c8f upstream. + +Kuee reported a quirk in the jmp32's jeq/jne simulation, namely that the +register value does not match expectations for the fall-through path. For +example: + +Before fix: + + 0: R1=ctx(off=0,imm=0) R10=fp0 + 0: (b7) r2 = 0 ; R2_w=P0 + 1: (b7) r6 = 563 ; R6_w=P563 + 2: (87) r2 = -r2 ; R2_w=Pscalar() + 3: (87) r2 = -r2 ; R2_w=Pscalar() + 4: (4c) w2 |= w6 ; R2_w=Pscalar(umin=563,umax=4294967295,var_off=(0x233; 0xfffffdcc),s32_min=-2147483085) R6_w=P563 + 5: (56) if w2 != 0x8 goto pc+1 ; R2_w=P571 <--- [*] + 6: (95) exit + R0 !read_ok + +After fix: + + 0: R1=ctx(off=0,imm=0) R10=fp0 + 0: (b7) r2 = 0 ; R2_w=P0 + 1: (b7) r6 = 563 ; R6_w=P563 + 2: (87) r2 = -r2 ; R2_w=Pscalar() + 3: (87) r2 = -r2 ; R2_w=Pscalar() + 4: (4c) w2 |= w6 ; R2_w=Pscalar(umin=563,umax=4294967295,var_off=(0x233; 0xfffffdcc),s32_min=-2147483085) R6_w=P563 + 5: (56) if w2 != 0x8 goto pc+1 ; R2_w=P8 <--- [*] + 6: (95) exit + R0 !read_ok + +As can be seen on line 5 for the branch fall-through path in R2 [*] is that +given condition w2 != 0x8 is false, verifier should conclude that r2 = 8 as +upper 32 bit are known to be zero. However, verifier incorrectly concludes +that r2 = 571 which is far off. + +The problem is it only marks false{true}_reg as known in the switch for JE/NE +case, but at the end of the function, it uses {false,true}_{64,32}off to +update {false,true}_reg->var_off and they still hold the prior value of +{false,true}_reg->var_off before it got marked as known. The subsequent +__reg_combine_32_into_64() then propagates this old var_off and derives new +bounds. The information between min/max bounds on {false,true}_reg from +setting the register to known const combined with the {false,true}_reg->var_off +based on the old information then derives wrong register data. + +Fix it by detangling the BPF_JEQ/BPF_JNE cases and updating relevant +{false,true}_{64,32}off tnums along with the register marking to known +constant. + +Fixes: 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking") +Reported-by: Kuee K1r0a +Signed-off-by: Daniel Borkmann +Signed-off-by: Andrii Nakryiko +Acked-by: John Fastabend +Link: https://lore.kernel.org/bpf/20220701124727.11153-1-daniel@iogearbox.net +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 41 ++++++++++++++++++++++++----------------- + 1 file changed, 24 insertions(+), 17 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -7512,26 +7512,33 @@ static void reg_set_min_max(struct bpf_r + return; + + switch (opcode) { ++ /* JEQ/JNE comparison doesn't change the register equivalence. ++ * ++ * r1 = r2; ++ * if (r1 == 42) goto label; ++ * ... ++ * label: // here both r1 and r2 are known to be 42. ++ * ++ * Hence when marking register as known preserve it's ID. ++ */ + case BPF_JEQ: ++ if (is_jmp32) { ++ __mark_reg32_known(true_reg, val32); ++ true_32off = tnum_subreg(true_reg->var_off); ++ } else { ++ ___mark_reg_known(true_reg, val); ++ true_64off = true_reg->var_off; ++ } ++ break; + case BPF_JNE: +- { +- struct bpf_reg_state *reg = +- opcode == BPF_JEQ ? true_reg : false_reg; +- +- /* JEQ/JNE comparison doesn't change the register equivalence. +- * r1 = r2; +- * if (r1 == 42) goto label; +- * ... +- * label: // here both r1 and r2 are known to be 42. +- * +- * Hence when marking register as known preserve it's ID. +- */ +- if (is_jmp32) +- __mark_reg32_known(reg, val32); +- else +- ___mark_reg_known(reg, val); ++ if (is_jmp32) { ++ __mark_reg32_known(false_reg, val32); ++ false_32off = tnum_subreg(false_reg->var_off); ++ } else { ++ ___mark_reg_known(false_reg, val); ++ false_64off = false_reg->var_off; ++ } + break; +- } + case BPF_JSET: + if (is_jmp32) { + false_32off = tnum_and(false_32off, tnum_const(~val32)); diff --git a/queue-5.10/bpf-fix-insufficient-bounds-propagation-from-adjust_scalar_min_max_vals.patch b/queue-5.10/bpf-fix-insufficient-bounds-propagation-from-adjust_scalar_min_max_vals.patch new file mode 100644 index 00000000000..fcd9879e0be --- /dev/null +++ b/queue-5.10/bpf-fix-insufficient-bounds-propagation-from-adjust_scalar_min_max_vals.patch @@ -0,0 +1,216 @@ +From 3844d153a41adea718202c10ae91dc96b37453b5 Mon Sep 17 00:00:00 2001 +From: Daniel Borkmann +Date: Fri, 1 Jul 2022 14:47:25 +0200 +Subject: bpf: Fix insufficient bounds propagation from adjust_scalar_min_max_vals + +From: Daniel Borkmann + +commit 3844d153a41adea718202c10ae91dc96b37453b5 upstream. + +Kuee reported a corner case where the tnum becomes constant after the call +to __reg_bound_offset(), but the register's bounds are not, that is, its +min bounds are still not equal to the register's max bounds. + +This in turn allows to leak pointers through turning a pointer register as +is into an unknown scalar via adjust_ptr_min_max_vals(). + +Before: + + func#0 @0 + 0: R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 0: (b7) r0 = 1 ; R0_w=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) + 1: (b7) r3 = 0 ; R3_w=scalar(imm=0,umax=0,var_off=(0x0; 0x0)) + 2: (87) r3 = -r3 ; R3_w=scalar() + 3: (87) r3 = -r3 ; R3_w=scalar() + 4: (47) r3 |= 32767 ; R3_w=scalar(smin=-9223372036854743041,umin=32767,var_off=(0x7fff; 0xffffffffffff8000),s32_min=-2147450881) + 5: (75) if r3 s>= 0x0 goto pc+1 ; R3_w=scalar(umin=9223372036854808575,var_off=(0x8000000000007fff; 0x7fffffffffff8000),s32_min=-2147450881,u32_min=32767) + 6: (95) exit + + from 5 to 7: R0=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R3=scalar(umin=32767,umax=9223372036854775807,var_off=(0x7fff; 0x7fffffffffff8000),s32_min=-2147450881) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 7: (d5) if r3 s<= 0x8000 goto pc+1 ; R3=scalar(umin=32769,umax=9223372036854775807,var_off=(0x7fff; 0x7fffffffffff8000),s32_min=-2147450881,u32_min=32767) + 8: (95) exit + + from 7 to 9: R0=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R3=scalar(umin=32767,umax=32768,var_off=(0x7fff; 0x8000)) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 9: (07) r3 += -32767 ; R3_w=scalar(imm=0,umax=1,var_off=(0x0; 0x0)) <--- [*] + 10: (95) exit + +What can be seen here is that R3=scalar(umin=32767,umax=32768,var_off=(0x7fff; +0x8000)) after the operation R3 += -32767 results in a 'malformed' constant, that +is, R3_w=scalar(imm=0,umax=1,var_off=(0x0; 0x0)). Intersecting with var_off has +not been done at that point via __update_reg_bounds(), which would have improved +the umax to be equal to umin. + +Refactor the tnum <> min/max bounds information flow into a reg_bounds_sync() +helper and use it consistently everywhere. After the fix, bounds have been +corrected to R3_w=scalar(imm=0,umax=0,var_off=(0x0; 0x0)) and thus the register +is regarded as a 'proper' constant scalar of 0. + +After: + + func#0 @0 + 0: R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 0: (b7) r0 = 1 ; R0_w=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) + 1: (b7) r3 = 0 ; R3_w=scalar(imm=0,umax=0,var_off=(0x0; 0x0)) + 2: (87) r3 = -r3 ; R3_w=scalar() + 3: (87) r3 = -r3 ; R3_w=scalar() + 4: (47) r3 |= 32767 ; R3_w=scalar(smin=-9223372036854743041,umin=32767,var_off=(0x7fff; 0xffffffffffff8000),s32_min=-2147450881) + 5: (75) if r3 s>= 0x0 goto pc+1 ; R3_w=scalar(umin=9223372036854808575,var_off=(0x8000000000007fff; 0x7fffffffffff8000),s32_min=-2147450881,u32_min=32767) + 6: (95) exit + + from 5 to 7: R0=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R3=scalar(umin=32767,umax=9223372036854775807,var_off=(0x7fff; 0x7fffffffffff8000),s32_min=-2147450881) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 7: (d5) if r3 s<= 0x8000 goto pc+1 ; R3=scalar(umin=32769,umax=9223372036854775807,var_off=(0x7fff; 0x7fffffffffff8000),s32_min=-2147450881,u32_min=32767) + 8: (95) exit + + from 7 to 9: R0=scalar(imm=1,umin=1,umax=1,var_off=(0x1; 0x0)) R1=ctx(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) R3=scalar(umin=32767,umax=32768,var_off=(0x7fff; 0x8000)) R10=fp(off=0,imm=0,umax=0,var_off=(0x0; 0x0)) + 9: (07) r3 += -32767 ; R3_w=scalar(imm=0,umax=0,var_off=(0x0; 0x0)) <--- [*] + 10: (95) exit + +Fixes: b03c9f9fdc37 ("bpf/verifier: track signed and unsigned min/max values") +Reported-by: Kuee K1r0a +Signed-off-by: Daniel Borkmann +Signed-off-by: Andrii Nakryiko +Acked-by: John Fastabend +Link: https://lore.kernel.org/bpf/20220701124727.11153-2-daniel@iogearbox.net +Signed-off-by: Greg Kroah-Hartman +--- + kernel/bpf/verifier.c | 72 +++++++++++++++----------------------------------- + 1 file changed, 23 insertions(+), 49 deletions(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -1249,6 +1249,21 @@ static void __reg_bound_offset(struct bp + reg->var_off = tnum_or(tnum_clear_subreg(var64_off), var32_off); + } + ++static void reg_bounds_sync(struct bpf_reg_state *reg) ++{ ++ /* We might have learned new bounds from the var_off. */ ++ __update_reg_bounds(reg); ++ /* We might have learned something about the sign bit. */ ++ __reg_deduce_bounds(reg); ++ /* We might have learned some bits from the bounds. */ ++ __reg_bound_offset(reg); ++ /* Intersecting with the old var_off might have improved our bounds ++ * slightly, e.g. if umax was 0x7f...f and var_off was (0; 0xf...fc), ++ * then new var_off is (0; 0x7f...fc) which improves our umax. ++ */ ++ __update_reg_bounds(reg); ++} ++ + static bool __reg32_bound_s64(s32 a) + { + return a >= 0 && a <= S32_MAX; +@@ -1290,16 +1305,8 @@ static void __reg_combine_32_into_64(str + * so they do not impact tnum bounds calculation. + */ + __mark_reg64_unbounded(reg); +- __update_reg_bounds(reg); + } +- +- /* Intersecting with the old var_off might have improved our bounds +- * slightly. e.g. if umax was 0x7f...f and var_off was (0; 0xf...fc), +- * then new var_off is (0; 0x7f...fc) which improves our umax. +- */ +- __reg_deduce_bounds(reg); +- __reg_bound_offset(reg); +- __update_reg_bounds(reg); ++ reg_bounds_sync(reg); + } + + static bool __reg64_bound_s32(s64 a) +@@ -1315,7 +1322,6 @@ static bool __reg64_bound_u32(u64 a) + static void __reg_combine_64_into_32(struct bpf_reg_state *reg) + { + __mark_reg32_unbounded(reg); +- + if (__reg64_bound_s32(reg->smin_value) && __reg64_bound_s32(reg->smax_value)) { + reg->s32_min_value = (s32)reg->smin_value; + reg->s32_max_value = (s32)reg->smax_value; +@@ -1324,14 +1330,7 @@ static void __reg_combine_64_into_32(str + reg->u32_min_value = (u32)reg->umin_value; + reg->u32_max_value = (u32)reg->umax_value; + } +- +- /* Intersecting with the old var_off might have improved our bounds +- * slightly. e.g. if umax was 0x7f...f and var_off was (0; 0xf...fc), +- * then new var_off is (0; 0x7f...fc) which improves our umax. +- */ +- __reg_deduce_bounds(reg); +- __reg_bound_offset(reg); +- __update_reg_bounds(reg); ++ reg_bounds_sync(reg); + } + + /* Mark a register as having a completely unknown (scalar) value. */ +@@ -5230,9 +5229,7 @@ static void do_refine_retval_range(struc + ret_reg->s32_max_value = meta->msize_max_value; + ret_reg->smin_value = -MAX_ERRNO; + ret_reg->s32_min_value = -MAX_ERRNO; +- __reg_deduce_bounds(ret_reg); +- __reg_bound_offset(ret_reg); +- __update_reg_bounds(ret_reg); ++ reg_bounds_sync(ret_reg); + } + + static int +@@ -6197,11 +6194,7 @@ reject: + + if (!check_reg_sane_offset(env, dst_reg, ptr_reg->type)) + return -EINVAL; +- +- __update_reg_bounds(dst_reg); +- __reg_deduce_bounds(dst_reg); +- __reg_bound_offset(dst_reg); +- ++ reg_bounds_sync(dst_reg); + if (sanitize_check_bounds(env, insn, dst_reg) < 0) + return -EACCES; + if (sanitize_needed(opcode)) { +@@ -6939,10 +6932,7 @@ static int adjust_scalar_min_max_vals(st + /* ALU32 ops are zero extended into 64bit register */ + if (alu32) + zext_32_to_64(dst_reg); +- +- __update_reg_bounds(dst_reg); +- __reg_deduce_bounds(dst_reg); +- __reg_bound_offset(dst_reg); ++ reg_bounds_sync(dst_reg); + return 0; + } + +@@ -7131,10 +7121,7 @@ static int check_alu_op(struct bpf_verif + insn->dst_reg); + } + zext_32_to_64(dst_reg); +- +- __update_reg_bounds(dst_reg); +- __reg_deduce_bounds(dst_reg); +- __reg_bound_offset(dst_reg); ++ reg_bounds_sync(dst_reg); + } + } else { + /* case: R = imm +@@ -7693,21 +7680,8 @@ static void __reg_combine_min_max(struct + dst_reg->smax_value); + src_reg->var_off = dst_reg->var_off = tnum_intersect(src_reg->var_off, + dst_reg->var_off); +- /* We might have learned new bounds from the var_off. */ +- __update_reg_bounds(src_reg); +- __update_reg_bounds(dst_reg); +- /* We might have learned something about the sign bit. */ +- __reg_deduce_bounds(src_reg); +- __reg_deduce_bounds(dst_reg); +- /* We might have learned some bits from the bounds. */ +- __reg_bound_offset(src_reg); +- __reg_bound_offset(dst_reg); +- /* Intersecting with the old var_off might have improved our bounds +- * slightly. e.g. if umax was 0x7f...f and var_off was (0; 0xf...fc), +- * then new var_off is (0; 0x7f...fc) which improves our umax. +- */ +- __update_reg_bounds(src_reg); +- __update_reg_bounds(dst_reg); ++ reg_bounds_sync(src_reg); ++ reg_bounds_sync(dst_reg); + } + + static void reg_combine_min_max(struct bpf_reg_state *true_src, diff --git a/queue-5.10/net-rose-fix-uaf-bug-caused-by-rose_t0timer_expiry.patch b/queue-5.10/net-rose-fix-uaf-bug-caused-by-rose_t0timer_expiry.patch new file mode 100644 index 00000000000..ff4c565130b --- /dev/null +++ b/queue-5.10/net-rose-fix-uaf-bug-caused-by-rose_t0timer_expiry.patch @@ -0,0 +1,73 @@ +From 148ca04518070910739dfc4eeda765057856403d Mon Sep 17 00:00:00 2001 +From: Duoming Zhou +Date: Tue, 5 Jul 2022 20:56:10 +0800 +Subject: net: rose: fix UAF bug caused by rose_t0timer_expiry + +From: Duoming Zhou + +commit 148ca04518070910739dfc4eeda765057856403d upstream. + +There are UAF bugs caused by rose_t0timer_expiry(). The +root cause is that del_timer() could not stop the timer +handler that is running and there is no synchronization. +One of the race conditions is shown below: + + (thread 1) | (thread 2) + | rose_device_event + | rose_rt_device_down + | rose_remove_neigh +rose_t0timer_expiry | rose_stop_t0timer(rose_neigh) + ... | del_timer(&neigh->t0timer) + | kfree(rose_neigh) //[1]FREE + neigh->dce_mode //[2]USE | + +The rose_neigh is deallocated in position [1] and use in +position [2]. + +The crash trace triggered by POC is like below: + +BUG: KASAN: use-after-free in expire_timers+0x144/0x320 +Write of size 8 at addr ffff888009b19658 by task swapper/0/0 +... +Call Trace: + + dump_stack_lvl+0xbf/0xee + print_address_description+0x7b/0x440 + print_report+0x101/0x230 + ? expire_timers+0x144/0x320 + kasan_report+0xed/0x120 + ? expire_timers+0x144/0x320 + expire_timers+0x144/0x320 + __run_timers+0x3ff/0x4d0 + run_timer_softirq+0x41/0x80 + __do_softirq+0x233/0x544 + ... + +This patch changes rose_stop_ftimer() and rose_stop_t0timer() +in rose_remove_neigh() to del_timer_sync() in order that the +timer handler could be finished before the resources such as +rose_neigh and so on are deallocated. As a result, the UAF +bugs could be mitigated. + +Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") +Signed-off-by: Duoming Zhou +Link: https://lore.kernel.org/r/20220705125610.77971-1-duoming@zju.edu.cn +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + net/rose/rose_route.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +--- a/net/rose/rose_route.c ++++ b/net/rose/rose_route.c +@@ -227,8 +227,8 @@ static void rose_remove_neigh(struct ros + { + struct rose_neigh *s; + +- rose_stop_ftimer(rose_neigh); +- rose_stop_t0timer(rose_neigh); ++ del_timer_sync(&rose_neigh->ftimer); ++ del_timer_sync(&rose_neigh->t0timer); + + skb_queue_purge(&rose_neigh->queue); + diff --git a/queue-5.10/netfilter-nf_tables-stricter-validation-of-element-data.patch b/queue-5.10/netfilter-nf_tables-stricter-validation-of-element-data.patch new file mode 100644 index 00000000000..9538884d446 --- /dev/null +++ b/queue-5.10/netfilter-nf_tables-stricter-validation-of-element-data.patch @@ -0,0 +1,44 @@ +From 7e6bc1f6cabcd30aba0b11219d8e01b952eacbb6 Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +Date: Sat, 2 Jul 2022 04:16:30 +0200 +Subject: netfilter: nf_tables: stricter validation of element data + +From: Pablo Neira Ayuso + +commit 7e6bc1f6cabcd30aba0b11219d8e01b952eacbb6 upstream. + +Make sure element data type and length do not mismatch the one specified +by the set declaration. + +Fixes: 7d7402642eaf ("netfilter: nf_tables: variable sized set element keys / data") +Reported-by: Hugues ANGUELKOV +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Greg Kroah-Hartman +--- + net/netfilter/nf_tables_api.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +--- a/net/netfilter/nf_tables_api.c ++++ b/net/netfilter/nf_tables_api.c +@@ -4886,13 +4886,20 @@ static int nft_setelem_parse_data(struct + struct nft_data *data, + struct nlattr *attr) + { ++ u32 dtype; + int err; + + err = nft_data_init(ctx, data, NFT_DATA_VALUE_MAXLEN, desc, attr); + if (err < 0) + return err; + +- if (desc->type != NFT_DATA_VERDICT && desc->len != set->dlen) { ++ if (set->dtype == NFT_DATA_VERDICT) ++ dtype = NFT_DATA_VERDICT; ++ else ++ dtype = NFT_DATA_VALUE; ++ ++ if (dtype != desc->type || ++ set->dlen != desc->len) { + nft_data_release(data, desc->type); + return -EINVAL; + } diff --git a/queue-5.10/netfilter-nft_set_pipapo-release-elements-in-clone-from-abort-path.patch b/queue-5.10/netfilter-nft_set_pipapo-release-elements-in-clone-from-abort-path.patch new file mode 100644 index 00000000000..cd8bb5a59da --- /dev/null +++ b/queue-5.10/netfilter-nft_set_pipapo-release-elements-in-clone-from-abort-path.patch @@ -0,0 +1,123 @@ +From 9827a0e6e23bf43003cd3d5b7fb11baf59a35e1e Mon Sep 17 00:00:00 2001 +From: Pablo Neira Ayuso +Date: Sat, 2 Jul 2022 04:16:31 +0200 +Subject: netfilter: nft_set_pipapo: release elements in clone from abort path + +From: Pablo Neira Ayuso + +commit 9827a0e6e23bf43003cd3d5b7fb11baf59a35e1e upstream. + +New elements that reside in the clone are not released in case that the +transaction is aborted. + +[16302.231754] ------------[ cut here ]------------ +[16302.231756] WARNING: CPU: 0 PID: 100509 at net/netfilter/nf_tables_api.c:1864 nf_tables_chain_destroy+0x26/0x127 [nf_tables] +[...] +[16302.231882] CPU: 0 PID: 100509 Comm: nft Tainted: G W 5.19.0-rc3+ #155 +[...] +[16302.231887] RIP: 0010:nf_tables_chain_destroy+0x26/0x127 [nf_tables] +[16302.231899] Code: f3 fe ff ff 41 55 41 54 55 53 48 8b 6f 10 48 89 fb 48 c7 c7 82 96 d9 a0 8b 55 50 48 8b 75 58 e8 de f5 92 e0 83 7d 50 00 74 09 <0f> 0b 5b 5d 41 5c 41 5d c3 4c 8b 65 00 48 8b 7d 08 49 39 fc 74 05 +[...] +[16302.231917] Call Trace: +[16302.231919] +[16302.231921] __nf_tables_abort.cold+0x23/0x28 [nf_tables] +[16302.231934] nf_tables_abort+0x30/0x50 [nf_tables] +[16302.231946] nfnetlink_rcv_batch+0x41a/0x840 [nfnetlink] +[16302.231952] ? __nla_validate_parse+0x48/0x190 +[16302.231959] nfnetlink_rcv+0x110/0x129 [nfnetlink] +[16302.231963] netlink_unicast+0x211/0x340 +[16302.231969] netlink_sendmsg+0x21e/0x460 + +Add nft_set_pipapo_match_destroy() helper function to release the +elements in the lookup tables. + +Stefano Brivio says: "We additionally look for elements pointers in the +cloned matching data if priv->dirty is set, because that means that +cloned data might point to additional elements we did not commit to the +working copy yet (such as the abort path case, but perhaps not limited +to it)." + +Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges") +Reviewed-by: Stefano Brivio +Signed-off-by: Pablo Neira Ayuso +Signed-off-by: Greg Kroah-Hartman +--- + net/netfilter/nft_set_pipapo.c | 48 ++++++++++++++++++++++++++++------------- + 1 file changed, 33 insertions(+), 15 deletions(-) + +--- a/net/netfilter/nft_set_pipapo.c ++++ b/net/netfilter/nft_set_pipapo.c +@@ -2121,6 +2121,32 @@ out_scratch: + } + + /** ++ * nft_set_pipapo_match_destroy() - Destroy elements from key mapping array ++ * @set: nftables API set representation ++ * @m: matching data pointing to key mapping array ++ */ ++static void nft_set_pipapo_match_destroy(const struct nft_set *set, ++ struct nft_pipapo_match *m) ++{ ++ struct nft_pipapo_field *f; ++ int i, r; ++ ++ for (i = 0, f = m->f; i < m->field_count - 1; i++, f++) ++ ; ++ ++ for (r = 0; r < f->rules; r++) { ++ struct nft_pipapo_elem *e; ++ ++ if (r < f->rules - 1 && f->mt[r + 1].e == f->mt[r].e) ++ continue; ++ ++ e = f->mt[r].e; ++ ++ nft_set_elem_destroy(set, e, true); ++ } ++} ++ ++/** + * nft_pipapo_destroy() - Free private data for set and all committed elements + * @set: nftables API set representation + */ +@@ -2128,26 +2154,13 @@ static void nft_pipapo_destroy(const str + { + struct nft_pipapo *priv = nft_set_priv(set); + struct nft_pipapo_match *m; +- struct nft_pipapo_field *f; +- int i, r, cpu; ++ int cpu; + + m = rcu_dereference_protected(priv->match, true); + if (m) { + rcu_barrier(); + +- for (i = 0, f = m->f; i < m->field_count - 1; i++, f++) +- ; +- +- for (r = 0; r < f->rules; r++) { +- struct nft_pipapo_elem *e; +- +- if (r < f->rules - 1 && f->mt[r + 1].e == f->mt[r].e) +- continue; +- +- e = f->mt[r].e; +- +- nft_set_elem_destroy(set, e, true); +- } ++ nft_set_pipapo_match_destroy(set, m); + + #ifdef NFT_PIPAPO_ALIGN + free_percpu(m->scratch_aligned); +@@ -2161,6 +2174,11 @@ static void nft_pipapo_destroy(const str + } + + if (priv->clone) { ++ m = priv->clone; ++ ++ if (priv->dirty) ++ nft_set_pipapo_match_destroy(set, m); ++ + #ifdef NFT_PIPAPO_ALIGN + free_percpu(priv->clone->scratch_aligned); + #endif diff --git a/queue-5.10/series b/queue-5.10/series index bd235bcb755..806d7a36897 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -3,3 +3,9 @@ alsa-hda-realtek-add-quirk-for-clevo-l140pu.patch can-bcm-use-call_rcu-instead-of-costly-synchronize_rcu.patch can-grcan-grcan_probe-remove-extra-of_node_get.patch can-gs_usb-gs_usb_open-close-fix-memory-leak.patch +bpf-fix-incorrect-verifier-simulation-around-jmp32-s-jeq-jne.patch +bpf-fix-insufficient-bounds-propagation-from-adjust_scalar_min_max_vals.patch +usbnet-fix-memory-leak-in-error-case.patch +net-rose-fix-uaf-bug-caused-by-rose_t0timer_expiry.patch +netfilter-nft_set_pipapo-release-elements-in-clone-from-abort-path.patch +netfilter-nf_tables-stricter-validation-of-element-data.patch diff --git a/queue-5.10/usbnet-fix-memory-leak-in-error-case.patch b/queue-5.10/usbnet-fix-memory-leak-in-error-case.patch new file mode 100644 index 00000000000..278c7b74c1b --- /dev/null +++ b/queue-5.10/usbnet-fix-memory-leak-in-error-case.patch @@ -0,0 +1,69 @@ +From b55a21b764c1e182014630fa5486d717484ac58f Mon Sep 17 00:00:00 2001 +From: Oliver Neukum +Date: Tue, 5 Jul 2022 14:53:51 +0200 +Subject: usbnet: fix memory leak in error case + +From: Oliver Neukum + +commit b55a21b764c1e182014630fa5486d717484ac58f upstream. + +usbnet_write_cmd_async() mixed up which buffers +need to be freed in which error case. + +v2: add Fixes tag +v3: fix uninitialized buf pointer + +Fixes: 877bd862f32b8 ("usbnet: introduce usbnet 3 command helpers") +Signed-off-by: Oliver Neukum +Link: https://lore.kernel.org/r/20220705125351.17309-1-oneukum@suse.com +Signed-off-by: Jakub Kicinski +Signed-off-by: Greg Kroah-Hartman +--- + drivers/net/usb/usbnet.c | 17 ++++++++++++----- + 1 file changed, 12 insertions(+), 5 deletions(-) + +--- a/drivers/net/usb/usbnet.c ++++ b/drivers/net/usb/usbnet.c +@@ -2102,7 +2102,7 @@ static void usbnet_async_cmd_cb(struct u + int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype, + u16 value, u16 index, const void *data, u16 size) + { +- struct usb_ctrlrequest *req = NULL; ++ struct usb_ctrlrequest *req; + struct urb *urb; + int err = -ENOMEM; + void *buf = NULL; +@@ -2120,7 +2120,7 @@ int usbnet_write_cmd_async(struct usbnet + if (!buf) { + netdev_err(dev->net, "Error allocating buffer" + " in %s!\n", __func__); +- goto fail_free; ++ goto fail_free_urb; + } + } + +@@ -2144,14 +2144,21 @@ int usbnet_write_cmd_async(struct usbnet + if (err < 0) { + netdev_err(dev->net, "Error submitting the control" + " message: status=%d\n", err); +- goto fail_free; ++ goto fail_free_all; + } + return 0; + ++fail_free_all: ++ kfree(req); + fail_free_buf: + kfree(buf); +-fail_free: +- kfree(req); ++ /* ++ * avoid a double free ++ * needed because the flag can be set only ++ * after filling the URB ++ */ ++ urb->transfer_flags = 0; ++fail_free_urb: + usb_free_urb(urb); + fail: + return err;