potential buffer overrun in p54_rx_eeprom_readback()
Subsystems (3)
If a malicious USB device pretends to be an Intersil p54 wifi interface and generates an eeprom_readback message with a large eeprom->v1.len, p54_rx_eeprom_readback() will copy data from the message beyond the end of priv->eeprom.
static void p54_rx_eeprom_readback(struct p54_common *priv,
struct sk_buff *skb)
{
struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
struct p54_eeprom_lm86 *eeprom = (struct p54_eeprom_lm86 *) hdr->data;
if (priv->fw_var >= 0x509) {
memcpy(priv->eeprom, eeprom->v2.data,
le16_to_cpu(eeprom->v2.len));
} else {
memcpy(priv->eeprom, eeprom->v1.data,
le16_to_cpu(eeprom->v1.len));
}priv->eeprom is allocated in p54_read_eeprom() with length 0x2020.
I've attached a demo, using usbip, that puts 0xffff in eeprom->v1.len.
# uname -a
Linux xxx 6.15.0-rc5-00136-g9c69f8884904 #20 SMP PREEMPT_DYNAMIC Wed May 14 16:45:28 EDT 2025 x86_64 x86_64 x86_64 GNU/Linux
# cc usbis2c.c # ./a.out [ 144.779715] ieee80211 phy1: unexpected end of eeprom data. [ 144.780624] ieee80211 phy1: eeprom parse failed! [ 146.100021] p54usb 1-1:1.0: failed to initialize device (-61) [ 146.101718] BUG: kernel NULL pointer dereference, address: 0000000000000010 [ 146.102814] #PF: supervisor read access in kernel mode [ 146.103629] #PF: error_code(0x0000) - not-present page [ 146.104438] PGD 0 P4D 0 [ 146.104862] Oops: Oops: 0000 [#1] SMP PTI [ 146.105514] CPU: 2 UID: 0 PID: 448 Comm: systemd-udevd Not tainted 6.15.0-rc5-00136-g9c69f8884904 #20 PREEMPT(voluntary) [ 146.106265] Hardware name: FreeBSD BHYVE/BHYVE, BIOS 14.0 10/17/2021 [ 146.106608] RIP: 0010:selinux_socket_sendmsg+0x55/0xe0 [ 146.106926] Code: 00 00 48 89 d7 49 8b b0 90 02 00 00 48 c7 44 24 08 00 00 00 00 48 c7 44 24 10 00 00 00 00 48 c7 44 24 18 00 00 0 0 00 f3 48 ab <8b> 4e 10 83 f9 01 74 64 0f b6 3d f6 f2 10 02 40 80 ff 01 74 05 83 [ 146.107900] RSP: 0018:ffff8bea407e3c48 EFLAGS: 00010246 [ 146.108181] RAX: 0000000000000000 RBX: ffff89f60736b4c0 RCX: 0000000000000000 [ 146.108560] RDX: ffff8bea407e3c68 RSI: 0000000000000000 RDI: ffff8bea407e3ca0 [ 146.108947] RBP: ffff8bea407e3ea0 R08: ffff89f602643e00 R09: 000000007ffff000 [ 146.109332] R10: 000000007fffee68 R11: 00007ffffffff000 R12: 0000000000000199 [ 146.109715] R13: ffff8bea407e3cd8 R14: ffff89f60736b4c0 R15: ffff8bea407e3cd8 [ 146.110104] FS: 00007f47e33128c0(0000) GS:ffff89f973551000(0000) knlGS:0000000000000000 [ 146.110530] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 146.110837] CR2: 0000000000000010 CR3: 0000000107698001 CR4: 00000000003706f0 [ 146.111202] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 146.111564] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 146.111930] Call Trace: [ 146.112069] <TASK> [ 146.112190] security_socket_sendmsg+0x37/0x50 [ 146.112428] ____sys_sendmsg+0x235/0x330 [ 146.112639] ___sys_sendmsg+0x94/0xe0 [ 146.112843] ? stack_depot_save_flags+0x29/0x7f0 [ 146.113096] __sys_sendmsg+0x81/0xe0 [ 146.113289] do_syscall_64+0x9e/0x1a0 [ 146.113490] entry_SYSCALL_64_after_hwframe+0x77/0x7f
Robert Morris
[email protected]
PATCH 1/1 wifi: p54: prevent buffer-overflow in p54_rx_eeprom_readback()
Robert Morris reported:
|If a malicious USB device pretends to be an Intersil p54 wifi
|interface and generates an eeprom_readback message with a large
|eeprom->v1.len, p54_rx_eeprom_readback() will copy data from the
|message beyond the end of priv->eeprom.
|
|static void p54_rx_eeprom_readback(struct p54_common *priv,
| struct sk_buff *skb)
|{
| struct p54_hdr *hdr = (struct p54_hdr *) skb->data;
| struct p54_eeprom_lm86 *eeprom = (struct p54_eeprom_lm86 *) hdr->data;
|
| if (priv->fw_var >= 0x509) {
| memcpy(priv->eeprom, eeprom->v2.data,
| le16_to_cpu(eeprom->v2.len));
| } else {
| memcpy(priv->eeprom, eeprom->v1.data,
| le16_to_cpu(eeprom->v1.len));
| }
| [...]The eeprom->v{1,2}.len is set by the driver in p54_download_eeprom(). The device is supposed to provide the same length back to the driver. But yes, it's possible (like shown in the report) to alter the value to something that causes a crash/panic due to overrun.
This patch addresses the issue by adding the size to the common device context, so p54_rx_eeprom_readback no longer relies on possibly tampered values... That said, it also checks if the "firmware" altered the value and no longer copies them.
The one, small saving grace is: Before the driver tries to read the eeprom, it needs to upload >a< firmware. the vendor firmware has a proprietary license and as a reason, it is not present on most distributions by default.
---
First of all: Congratulation and Thank you! This is a great report. the reprod. is the icing on the cake...
I guess you found more issues with p54, right?
From what I see, the first patch that introduced the "unchecked" buffer overrun has a date of "Sep 1 22:48:51 2008 +0200"
I don't know who currently handles these patches, maybe netdev?
--- drivers/net/wireless/intersil/p54/fwio.c | 2 ++ drivers/net/wireless/intersil/p54/p54.h | 1 + drivers/net/wireless/intersil/p54/txrx.c | 13 +++++++++---- 3 files changed, 12 insertions(+), 4 deletions(-)
drivers/net/wireless/intersil/p54/fwio.c +2 -0
@@ -231,6 +231,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf,mutex_lock (&priv->eeprom_mutex); priv->eeprom = buf; + priv->eeprom_slice_size = len; eeprom_hdr =skb_put (skb, eeprom_hdr_size + len);if (priv->fw_var <0x509 ) {
@@ -253,6 +254,7 @@ int p54_download_eeprom(struct p54_common *priv, void *buf, ret = -EBUSY ; } priv->eeprom =NULL ; + priv->eeprom_slice_size =0 ;mutex_unlock (&priv->eeprom_mutex);return ret; }
drivers/net/wireless/intersil/p54/p54.h +1 -0
@@ -258,6 +258,7 @@ struct p54_common {/* eeprom handling */ void *eeprom; +size_t eeprom_slice_size;struct completion eeprom_comp;struct mutex eeprom_mutex; };
drivers/net/wireless/intersil/p54/txrx.c +9 -4
@@ -496,14 +496,19 @@ static void p54_rx_eeprom_readback(struct p54_common *priv,return ;if (priv->fw_var >=0x509 ) { -memcpy (priv->eeprom, eeprom->v2.data, -le16_to_cpu (eeprom->v2.len)); +if (le16_to_cpu (eeprom->v2.len) != priv->eeprom_slice_size) +return ; + +memcpy (priv->eeprom, eeprom->v2.data, priv->eeprom_slice_size); }else { -memcpy (priv->eeprom, eeprom->v1.data, -le16_to_cpu (eeprom->v1.len)); +if (le16_to_cpu (eeprom->v1.len) != priv->eeprom_slice_size) +return ; + +memcpy (priv->eeprom, eeprom->v1.data, priv->eeprom_slice_size); } priv->eeprom =NULL ; + priv->eeprom_slice_size =0 ; tmp =p54_find_and_unlink_skb (priv, hdr->req_id);dev_kfree_skb_any (tmp);complete (&priv->eeprom_comp);
Signature · 1 line
2.49.0