| 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
| ||
|
Message-ID: <fd3c66f1-4b16-4da9-8a9d-615332fd0c8e@molgen.mpg.de>
Date: Mon, 1 Dec 2025 14:22:29 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Guangshuo Li <lgs201920130244@...il.com>
Cc: Jesse Brandeburg <jbrandeburg@...udflare.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
"David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Florian Westphal <fw@...len.de>,
intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Tony Nguyen <tony.nguyen@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v2] e1000: fix OOB in
e1000_tbi_should_accept()
[Cc: Remove bouncing address of Jeffrey and update Jesse’s]
Am 01.12.25 um 14:18 schrieb Paul Menzel:
> Dear Guangshuo,
>
>
> Thank you for your patch.
>
> Am 01.12.25 um 04:40 schrieb Guangshuo Li:
>> In e1000_tbi_should_accept() we read the last byte of the frame via
>> 'data[length - 1]' to evaluate the TBI workaround. If the descriptor-
>> reported length is zero or larger than the actual RX buffer size, this
>> read goes out of bounds and can hit unrelated slab objects. The issue
>> is observed from the NAPI receive path (e1000_clean_rx_irq):
>>
>> ==================================================================
>> BUG: KASAN: slab-out-of-bounds in e1000_tbi_should_accept+0x610/0x790
>> Read of size 1 at addr ffff888014114e54 by task sshd/363
>>
>> CPU: 0 PID: 363 Comm: sshd Not tainted 5.18.0-rc1 #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
>> Call Trace:
>> <IRQ>
>> dump_stack_lvl+0x5a/0x74
>> print_address_description+0x7b/0x440
>> print_report+0x101/0x200
>> kasan_report+0xc1/0xf0
>> e1000_tbi_should_accept+0x610/0x790
>> e1000_clean_rx_irq+0xa8c/0x1110
>> e1000_clean+0xde2/0x3c10
>> __napi_poll+0x98/0x380
>> net_rx_action+0x491/0xa20
>> __do_softirq+0x2c9/0x61d
>> do_softirq+0xd1/0x120
>> </IRQ>
>> <TASK>
>> __local_bh_enable_ip+0xfe/0x130
>> ip_finish_output2+0x7d5/0xb00
>> __ip_queue_xmit+0xe24/0x1ab0
>> __tcp_transmit_skb+0x1bcb/0x3340
>> tcp_write_xmit+0x175d/0x6bd0
>> __tcp_push_pending_frames+0x7b/0x280
>> tcp_sendmsg_locked+0x2e4f/0x32d0
>> tcp_sendmsg+0x24/0x40
>> sock_write_iter+0x322/0x430
>> vfs_write+0x56c/0xa60
>> ksys_write+0xd1/0x190
>> do_syscall_64+0x43/0x90
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>> RIP: 0033:0x7f511b476b10
>> Code: 73 01 c3 48 8b 0d 88 d3 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 83 3d f9 2b 2c 00 00 75 10 b8 01 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 8e 9b 01 00 48 89 04 24
>> RSP: 002b:00007ffc9211d4e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
>> RAX: ffffffffffffffda RBX: 0000000000004024 RCX: 00007f511b476b10
>> RDX: 0000000000004024 RSI: 0000559a9385962c RDI: 0000000000000003
>> RBP: 0000559a9383a400 R08: fffffffffffffff0 R09: 0000000000004f00
>> R10: 0000000000000070 R11: 0000000000000246 R12: 0000000000000000
>> R13: 00007ffc9211d57f R14: 0000559a9347bde7 R15: 0000000000000003
>> </TASK>
>> Allocated by task 1:
>> __kasan_krealloc+0x131/0x1c0
>> krealloc+0x90/0xc0
>> add_sysfs_param+0xcb/0x8a0
>> kernel_add_sysfs_param+0x81/0xd4
>> param_sysfs_builtin+0x138/0x1a6
>> param_sysfs_init+0x57/0x5b
>> do_one_initcall+0x104/0x250
>> do_initcall_level+0x102/0x132
>> do_initcalls+0x46/0x74
>> kernel_init_freeable+0x28f/0x393
>> kernel_init+0x14/0x1a0
>> ret_from_fork+0x22/0x30
>> The buggy address belongs to the object at ffff888014114000
>> which belongs to the cache kmalloc-2k of size 2048
>> The buggy address is located 1620 bytes to the right of
>> 2048-byte region [ffff888014114000, ffff888014114800]
>> The buggy address belongs to the physical page:
>> page:ffffea0000504400 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x14110
>> head:ffffea0000504400 order:3 compound_mapcount:0 compound_pincount:0
>> flags: 0x100000000010200(slab|head|node=0|zone=1)
>> raw: 0100000000010200 0000000000000000 dead000000000001 ffff888013442000
>> raw: 0000000000000000 0000000000080008 00000001ffffffff 0000000000000000
>> page dumped because: kasan: bad access detected
>> ==================================================================
>>
>> This happens because the TBI check unconditionally dereferences the last
>> byte without validating the reported length first:
>>
>> u8 last_byte = *(data + length - 1);
>>
>> Fix by rejecting the frame early if the length is zero, or if it exceeds
>> adapter->rx_buffer_len. This preserves the TBI workaround semantics for
>> valid frames and prevents touching memory beyond the RX buffer.
>
> Do you have reproducer to forth an invalid length?
>
>> Fixes: 2037110c96d5 ("e1000: move tbi workaround code into helper function")
>> Cc: stable@...r.kernel.org
>> Suggested-by: Tony Nguyen <tony.nguyen@...el.com>
>> Signed-off-by: Guangshuo Li <lgs201920130244@...il.com>
>> ---
>> changelog:
>> v2:
>> - Keep declarations at the beginning of e1000_tbi_should_accept().
>> - Move the last_byte assignment after the length bounds checks
>> (suggested by Tony Nguyen)
>> ---
>> drivers/net/ethernet/intel/e1000/e1000_main.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
>> index 3f5feb55cfba..cb49ec49f836 100644
>> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
>> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
>> @@ -4091,7 +4091,14 @@ static bool e1000_tbi_should_accept(struct
>> e1000_adapter *adapter,
>> u32 length, const u8 *data)
>> {
>> struct e1000_hw *hw = &adapter->hw;
>> - u8 last_byte = *(data + length - 1);
>> + u8 last_byte;
>> + /* Guard against OOB on data[length - 1] */
>> + if (unlikely(!length))
>> + return false;
>> + /* Upper bound: length must not exceed rx_buffer_len */
>> + if (unlikely(length > adapter->rx_buffer_len))
>
> Should an error be logged, or is it a common scenario, that such traffic
> exists?
>
>> + return false;
>> + last_byte = *(data + length - 1);
>> if (TBI_ACCEPT(hw, status, errors, length, last_byte)) {
>> unsigned long irq_flags;
>
>
> Kind regards,
>
> Paul
Powered by blists - more mailing lists