Commit da1b9a55 authored by Christian Lamparter's avatar Christian Lamparter Committed by Johannes Berg
Browse files

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.

Cc: <stable@kernel.org>
Reported-by: default avatarRobert Morris <rtm@mit.edu>
Closes: https://lore.kernel.org/linux-wireless/28782.1747258414@localhost/


Fixes: 7cb77072 ("p54: move eeprom code into common library")
Signed-off-by: default avatarChristian Lamparter <chunkeey@gmail.com>
Link: https://patch.msgid.link/20250516184107.47794-1-chunkeey@gmail.com


Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
parent d7500fbf

drivers/net/wireless/intersil/p54/fwio.c

+2 −0
Original line number Diff line number Diff line
@@ -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
Original line number Diff line number Diff line
@@ -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
Original line number Diff line number Diff line
@@ -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);