netdev - Re: [PATCH net v2 2/2] net/sched: act_mirred: don't override retval if we already lost the skb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zc3I0YmE4vYrkvYz@mev-dev>
Date: Thu, 15 Feb 2024 09:18:25 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
	pabeni@...hat.com, jhs@...atatu.com, xiyou.wangcong@...il.com,
	jiri@...nulli.us
Subject: Re: [PATCH net v2 2/2] net/sched: act_mirred: don't override retval
 if we already lost the skb

On Tue, Feb 13, 2024 at 07:38:48PM -0800, Jakub Kicinski wrote:
> If we're redirecting the skb, and haven't called tcf_mirred_forward(),
> yet, we need to tell the core to drop the skb by setting the retcode
> to SHOT. If we have called tcf_mirred_forward(), however, the skb
> is out of our hands and returning SHOT will lead to UaF.
> 
Thanks for fixing it. I had this UaF after ingress to ingress
redirection (was wondering if it is reasonable filter configuration).

[ 1459.177197] refcount_t: underflow; use-after-free.
[ 1459.177219] WARNING: CPU: 32 PID: 2427 at lib/refcount.c:28 refcount_warn_saturate+0xba/0x110
[ 1459.177227] Modules linked in: act_mirred cls_flower sch_ingress veth irdma ice(E) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables nfnetlink ipmi_ssif intel_rapl_msr intel_rapl_common intel_uncore_frequency intel_uncore_frequency_common i10nm_edac nfit libnvdimm x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel mlx5_ib i40e kvm vfat fat ib_uverbs dell_wmi irqbypass ledtrig_audio rapl sparse_keymap acpi_ipmi iTCO_wdt rfkill intel_cstate intel_pmc_bxt video ipmi_si dell_smbios mei_me iTCO_vendor_support ib_core intel_uncore dcdbas joydev dell_wmi_descriptor wmi_bmof dax_hmem isst_if_mmio pcspkr isst_if_mbox_pci ipmi_devintf mei i2c_i801 isst_if_common intel_pch_thermal i2c_smbus ipmi_msghandler acpi_power_meter intel_vsec fuse zram mlx5_core crct10dif_pclmul crc32_pclmul crc32c_intel polyval_clmulni polyval_generic ghash_clmulni_intel mlxfw sha512_ssse3 bnxt_en tls
[ 1459.177300]  mgag200 sha256_ssse3 megaraid_sas tg3
[ 1459.177302] dev_queue_xmit: bootnet
[ 1459.177304]  sha1_ssse3 psample gnss pci_hyperv_intf
[ 1459.177309] sch_handle_egress on: bootnet
[ 1459.177312]  i2c_algo_bit wmi [last unloaded: ice]
[ 1459.177318] CPU: 32 PID: 2427 Comm: scapy Kdump: loaded Tainted: G            E      6.8.0-rc2+ #3
[ 1459.177321] Hardware name: Dell Inc. PowerEdge R750/0PJ80M, BIOS 1.12.1 09/13/2023
[ 1459.177322] RIP: 0010:refcount_warn_saturate+0xba/0x110
[ 1459.177326] Code: 01 01 e8 19 1f 92 ff 0f 0b c3 cc cc cc cc 80 3d fd 12 cf 01 00 75 85 48 c7 c7 70 de 93 89 c6 05 ed 12 cf 01 01 e8 f6 1e 92 ff <0f> 0b c3 cc cc cc cc 80 3d d8 12 cf 01 00 0f 85 5e ff ff ff 48 c7
[ 1459.177328] RSP: 0018:ff5e8ab706db4df0 EFLAGS: 00010282
[ 1459.177331] RAX: 0000000000000026 RBX: ff4f7131460b1000 RCX: 0000000000000000
[ 1459.177332] RDX: 0000000000000103 RSI: ffffffff89924668 RDI: 00000000ffffffff
[ 1459.177334] RBP: ff4f7138e904c000 R08: 0000000000000000 R09: ff5e8ab706db4c90
[ 1459.177335] R10: 0000000000000003 R11: ff4f7140bfd49b28 R12: 0000000000000000
[ 1459.177336] R13: ff4f7138e904c120 R14: 0000000000000002 R15: 0000000000000000
[ 1459.177337] FS:  00007f69ec975b80(0000) GS:ff4f7138a0200000(0000) knlGS:0000000000000000
[ 1459.177339] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1459.177340] CR2: 00007f6861c9d030 CR3: 0000000157fd0004 CR4: 0000000000771ef0
[ 1459.177342] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1459.177343] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1459.177344] PKRU: 55555554
[ 1459.177345] Call Trace:
[ 1459.177347]  <IRQ>
[ 1459.177348]  ? refcount_warn_saturate+0xba/0x110
[ 1459.177351]  ? __warn+0x7d/0x130
[ 1459.177358]  ? refcount_warn_saturate+0xba/0x110
[ 1459.177360]  ? report_bug+0x18d/0x1c0
[ 1459.177366]  ? prb_read_valid+0x17/0x20
[ 1459.177373]  ? handle_bug+0x41/0x70
[ 1459.177378]  ? exc_invalid_op+0x13/0x60
[ 1459.177382]  ? asm_exc_invalid_op+0x16/0x20
[ 1459.177388]  ? refcount_warn_saturate+0xba/0x110
[ 1459.177390]  skb_release_head_state+0x7d/0x90
[ 1459.177398]  kfree_skb_reason+0x35/0x110
[ 1459.177400]  __netif_receive_skb_core.constprop.0+0x971/0x1000
[ 1459.177406]  ? __blk_mq_free_request+0x70/0x100
[ 1459.177410]  ? blk_queue_exit+0xe/0x40
[ 1459.177416]  ? scsi_end_request+0xfb/0x1b0
[ 1459.177422]  __netif_receive_skb_one_core+0x2c/0x80
[ 1459.177424]  process_backlog+0x81/0x120
[ 1459.177427]  __napi_poll+0x28/0x1c0
[ 1459.177430]  net_rx_action+0x283/0x360
[ 1459.177432]  ? sched_clock+0xc/0x30
[ 1459.177438]  __do_softirq+0xf2/0x316
[ 1459.177443]  ? irqtime_account_irq+0xa4/0xd0
[ 1459.177448]  do_softirq.part.0+0x72/0x90
[ 1459.177452]  </IRQ>

After applying the patch it is fine.

> Move the retval override to the error path which actually need it.
> 
> Fixes: e5cf1baf92cb ("act_mirred: use TC_ACT_REINSERT when possible")
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
> CC: jhs@...atatu.com
> CC: xiyou.wangcong@...il.com
> CC: jiri@...nulli.us
> ---
>  net/sched/act_mirred.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 

Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>