[PATCH net v3 0/3] ieee802154: ca8210: fix stack and slab out-of-bounds accesses

[PATCH net v3 0/3] ieee802154: ca8210: fix stack and slab out-of-bounds accesses

[Hui Peng]

This series fixes three out-of-bounds access bugs in the Cascoda CA8210
IEEE 802.15.4 driver:

1. Reject received SPI packets with len > sizeof(struct mac_message) in
   ca8210_rx_done() instead of checking len > CA8210_SPI_BUF_SIZE (256),
   preventing a stack buffer overflow when copying a synchronous response
   into priv->sync_command_response (a struct mac_message on the caller's
   stack) and matching the actual SPI transfer length
   (cas_ctl->transfer.len = sizeof(struct mac_message)).
2. Initialize lenvar = 1 in ca8210_get_ed() and validate
   hw_attribute_length against *hw_attribute_length in
   hwme_get_request_sync() before memcpy() to prevent overflowing the
   caller's stack buffer.
3. Validate the received SPI frame length len upfront at the start of
   ca8210_skb_rx() before reading data_ind or allocating the skb.

Changes in v3:
- Patch 1/3: Check len > sizeof(struct mac_message) in ca8210_rx_done()
  where dev_crit() logs "Received packet len (%u) erroneously long" and
  drops the packet, instead of silently truncating the memcpy() with
  min_t(), addressing David Laight's feedback.

Changes in v2:
- Split the ca8210 fixes into three single-issue patches (1/3..3/3) and
  addressed Miquel Raynal's review comments on patches 2/3 and 3/3.

Hui Peng (3):
  ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()
  ieee802154: ca8210: prevent stack buffer overflow in
    hwme_get_request_sync()
  ieee802154: ca8210: validate data_ind length upfront in
    ca8210_skb_rx()

 drivers/net/ieee802154/ca8210.c | 39 +++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH net v3 1/3] ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

[Hui Peng]

In ca8210_spi_transfer(), each SPI transfer reads
sizeof(struct mac_message) bytes into cas_ctl->tx_in_buf:

  cas_ctl->transfer.len = sizeof(struct mac_message);

However, ca8210_rx_done() only checks the received packet length
len = buf[1] + 2 against CA8210_SPI_BUF_SIZE (256). When buf[0] & SPI_SYN
is set and priv->sync_command_response is non-NULL, memcpy() copies up to
256 bytes into priv->sync_command_response, which points to a
struct mac_message object on the synchronous caller's stack, overflowing
the stack buffer:

  BUG: KASAN: stack-out-of-bounds in ca8210_rx_done+0x117/0x6c0
  Write of size 256 at addr ffff8881009e7c40 by task swapper/0/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   kasan_check_range+0x125/0x200
   __asan_memcpy+0x3c/0x60
   ca8210_rx_done+0x117/0x6c0
  ...
  This frame has 1 object:
   [32, 182) 'response'

Check len > sizeof(struct mac_message) instead of
len > CA8210_SPI_BUF_SIZE in ca8210_rx_done() so that any packet exceeding
sizeof(struct mac_message) is logged as erroneously long via dev_crit()
and dropped before copying into priv->sync_command_response or passing it
to ca8210_net_rx().

Tested in QEMU with KASAN enabled by passing a 256-byte SPI_SYN response
into ca8210_rx_done().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v3:
- Check len > sizeof(struct mac_message) at the top of ca8210_rx_done()
  where dev_crit() logs the error and drops the packet instead of silently
  truncating memcpy() with min_t(), addressing David Laight's feedback.

Changes in v2:
- Split the ca8210 fixes into three single-issue patches (1/3..3/3).

 drivers/net/ieee802154/ca8210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 01af4f9..a990a0f 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -686,7 +686,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
 
 	buf = cas_ctl->tx_in_buf;
 	len = buf[1] + 2;
-	if (len > CA8210_SPI_BUF_SIZE) {
+	if (len > sizeof(struct mac_message)) {
 		dev_crit(
 			&priv->spi->dev,
 			"Received packet len (%u) erroneously long\n",
-- 
2.49.0

[PATCH net v3 2/3] ieee802154: ca8210: prevent stack buffer overflow in hwme_get_request_sync()

[Hui Peng]

In ca8210_get_ed(), an uninitialized u8 lenvar and a pointer to a 1-byte
stack buffer (u8 *level) are passed to hwme_get_request_sync(), which
unconditionally copies response.pdata.hwme_get_cnf.hw_attribute_length
bytes into hw_attribute_value without checking the caller's destination
buffer capacity, overflowing level on the stack when hw_attribute_length
exceeds 1:

  BUG: KASAN: stack-out-of-bounds in hwme_get_request_sync.constprop.0.isra.0+0xf3/0x170
  Write of size 16 at addr ffff888001907780 by task init/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   kasan_check_range+0x125/0x200
   __asan_memcpy+0x3c/0x60
   hwme_get_request_sync.constprop.0.isra.0+0xf3/0x170
   ca8210_get_ed+0x9c/0xf0
  ...
  The buggy address belongs to stack of task init/1
   and is located at offset 48 in frame:
   ca8210_get_ed+0x0/0xf0
  This frame has 2 objects:
   [48, 49) 'level'
   [64, 65) 'lenvar'

Initialize lenvar = 1 in ca8210_get_ed() and return
IEEE802154_SYSTEM_ERROR in hwme_get_request_sync() if
response.pdata.hwme_get_cnf.hw_attribute_length exceeds
*hw_attribute_length.

Tested in QEMU with KASAN enabled by passing an oversized
hw_attribute_length response into ca8210_get_ed().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v3:
- No changes.

Changes in v2:
- Split out as patch 2/3.
- Replaced the temporary stack buffer in ca8210_get_ed() with lenvar = 1
  and an upper-bound check against *hw_attribute_length in
  hwme_get_request_sync() as requested by Miquel Raynal.

 drivers/net/ieee802154/ca8210.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index a990a0f..8aa7ffe 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1677,6 +1677,9 @@ static u8 hwme_get_request_sync(
 		return IEEE802154_SYSTEM_ERROR;
 
 	if (response.pdata.hwme_get_cnf.status == IEEE802154_SUCCESS) {
+		if (response.pdata.hwme_get_cnf.hw_attribute_length >
+		    *hw_attribute_length)
+			return IEEE802154_SYSTEM_ERROR;
 		*hw_attribute_length =
 			response.pdata.hwme_get_cnf.hw_attribute_length;
 		memcpy(
@@ -2027,7 +2030,7 @@ static int ca8210_xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb)
  */
 static int ca8210_get_ed(struct ieee802154_hw *hw, u8 *level)
 {
-	u8 lenvar;
+	u8 lenvar = 1;
 	struct ca8210_priv *priv = hw->priv;
 
 	return link_to_linux_err(

[PATCH net v3 3/3] ieee802154: ca8210: validate data_ind length upfront in ca8210_skb_rx()

[Hui Peng]

In ca8210_skb_rx(), data_ind is read at offsets 22 (msdulen), 23
(mpdulinkquality), 29 + msdulen (hdr.sec.level), 30..39 + msdulen
(security header), and 29 .. 29 + msdulen (payload) without verifying
that the received SPI frame length len covers those offsets, causing an
out-of-bounds read when msdulen exceeds len - 30:

  BUG: KASAN: slab-out-of-bounds in ca8210_skb_rx.constprop.0.isra.0+0x137/0x160
  Read of size 64 at addr ffff888006453ddd by task init/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   kasan_check_range+0x125/0x200
   __asan_memcpy+0x23/0x60
   ca8210_skb_rx.constprop.0.isra.0+0x137/0x160
   ca8210_net_rx+0x96/0xc0
  ...
  The buggy address belongs to the object at ffff888006453dc0
   which belongs to the cache kmalloc-32 of size 32
  The buggy address is located 29 bytes inside of
   allocated 32-byte region [ffff888006453dc0, ffff888006453de0)

Consolidate all length and msdulen validations into a single upfront check
at the beginning of ca8210_skb_rx() before allocating the skb.

Tested in QEMU with KASAN enabled by passing a short data_ind buffer with
msdulen = 64 and len = 30 into ca8210_skb_rx().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v3:
- No changes.

Changes in v2:
- Split out as patch 3/3.
- Consolidated all length checks in ca8210_skb_rx() into a single place
  at the beginning of the function before dev_alloc_skb() and dropped the
  unrelated hdr.seq assignment as requested by Miquel Raynal.

 drivers/net/ieee802154/ca8210.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 8aa7ffe..ab245ad 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -1758,19 +1758,15 @@ static int ca8210_skb_rx(
 	u8                    *data_ind
 )
 {
-	struct ieee802154_hdr hdr;
+	struct ieee802154_hdr hdr = { };
 	int msdulen;
 	int hlen;
-	u8 mpdulinkquality = data_ind[23];
+	u8 mpdulinkquality;
 	struct sk_buff *skb;
 	struct ca8210_priv *priv = hw->priv;
 
-	/* Allocate mtu size buffer for every rx packet */
-	skb = dev_alloc_skb(IEEE802154_MTU + sizeof(hdr));
-	if (!skb)
-		return -ENOMEM;
-
-	skb_reserve(skb, sizeof(hdr));
+	if (len < 30)
+		return -EMSGSIZE;
 
 	msdulen = data_ind[22]; /* msdu_length */
 	if (msdulen > IEEE802154_MTU) {
@@ -1778,9 +1774,25 @@ static int ca8210_skb_rx(
 			&priv->spi->dev,
 			"received erroneously large msdu length!\n"
 		);
-		kfree_skb(skb);
 		return -EMSGSIZE;
 	}
+
+	if (len < 30 + msdulen ||
+	    (!priv->promiscuous && data_ind[29 + msdulen] > 0 &&
+	     len < 29 + msdulen + sizeof(struct secspec))) {
+		dev_err(&priv->spi->dev,
+			"received truncated data indication!\n");
+		return -EMSGSIZE;
+	}
+
+	mpdulinkquality = data_ind[23];
+
+	/* Allocate mtu size buffer for every rx packet */
+	skb = dev_alloc_skb(IEEE802154_MTU + sizeof(hdr));
+	if (!skb)
+		return -ENOMEM;
+
+	skb_reserve(skb, sizeof(hdr));
 	dev_dbg(&priv->spi->dev, "skb buffer length = %d\n", msdulen);
 
 	if (priv->promiscuous)

Re: [PATCH net v3 1/3] ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (2) · Medium: 0 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] ieee802154: ca8210: dropped Rx frames and log flood DoS due to
  length limit

Pre-existing issues:
- [High] priv->sync_command_response is a plain pointer to a synchronous
  caller's on-stack struct mac_message, and nothing synchronizes access…
- [High] ca8210_rx_done() calls msleep(2000) (line 718) and then
  kmalloc_obj(*mlme_reset_wpc) (line 723, default GFP_KERNEL) when it…
- [Low] Baseline decision: not reported (preexisting, low impact, outside
  the bug class the commit claims to fix).

--

ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

This patch changes the length check in ca8210_rx_done() so that it compares
against sizeof(struct mac_message) instead of CA8210_SPI_BUF_SIZE. A
response that is too long is now logged and dropped. It is no longer copied
into the synchronous caller's on-stack response buffer or passed to
ca8210_net_rx().

> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index 01af4f9cf7f29..a990a0f347ad8 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -686,7 +686,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
>  
>  	buf = cas_ctl->tx_in_buf;
>  	len = buf[1] + 2;
> -	if (len > CA8210_SPI_BUF_SIZE) {
> +	if (len > sizeof(struct mac_message)) {
>  		dev_crit(
>  			&priv->spi->dev,
>  			"Received packet len (%u) erroneously long\n",

[Severity: High]
Does this new limit also drop legitimate MCPS-DATA.indication frames?

The check sits at the top of ca8210_rx_done(), so it applies to every
message from the device. It is not limited to the synchronous responses
that get copied into priv->sync_command_response.

ca8210_net_rx() passes SPI_MCPS_DATA_INDICATION payloads to
ca8210_skb_rx() as buf + 2. ca8210_skb_rx() then reads the security
spec that follows the MSDU:

	hdr.sec.level = data_ind[29 + msdulen];
	...
	if (hdr.sec.level > 0) {
		hdr.sec.key_id_mode = data_ind[30 + msdulen];
		memcpy(&hdr.sec.extended_src, &data_ind[31 + msdulen], 8);
		hdr.sec.key_id = data_ind[39 + msdulen];
	}

That makes a full indication 42 + msdulen bytes. With a 127-byte MSDU
(IEEE802154_MTU), the device reports len = 169. That is larger than
sizeof(struct mac_message), which is 150 according to the KASAN report
in the changelog. If I read this right, any received frame with more
than 108 bytes of payload is now discarded.

Each dropped frame also triggers a dev_crit() with no rate limiting.
Anyone in radio range can send long frames, so a remote peer could
flood the kernel log this way. Should this at least use
dev_crit_ratelimited() or dev_err_ratelimited()?

ca8210_spi_transfer() only clocks in sizeof(struct mac_message) bytes.
So before this patch, the tail of a long indication may already have
come from stale tx_in_buf contents.

Would it be better to keep the generic limit here, and check against
sizeof(struct mac_message) only before the memcpy() into
priv->sync_command_response? Or should the transfer length be sized to
cover the largest MCPS-DATA.indication? Either way, dropping long data
frames on stable kernels looks like a regression worth avoiding.

[Severity: High]
This is a pre-existing issue, but since this patch is about the memcpy()
into priv->sync_command_response, can the pointer itself go stale?

Later in ca8210_rx_done(), the pointer is checked and then written through.
No lock or barrier protects it:

drivers/net/ieee802154/ca8210.c:ca8210_rx_done() {
    ...
	if (buf[0] & SPI_SYN) {
		if (priv->sync_command_response) {
			memcpy(priv->sync_command_response, buf, len);
			complete(&priv->sync_exchange_complete);
    ...
}

ca8210_spi_exchange() stores a pointer to the caller's stack buffer. It
then waits with an interruptible timeout. On timeout or signal, it clears
the pointer and returns:

drivers/net/ieee802154/ca8210.c:ca8210_spi_exchange() {
    ...
		priv->sync_command_response = response;
    ...
	wait_remaining = wait_for_completion_interruptible_timeout(
		&priv->sync_exchange_complete,
		msecs_to_jiffies(CA8210_SYNC_TIMEOUT)
	);
    ...
cleanup:
	priv->sync_command_response = NULL;
	return status;
}

Suppose the device response arrives just as the waiter times out or gets a
signal. Could this interleaving happen?

CPU1 (SPI completion)
ca8210_spi_transfer_complete()
  ca8210_rx_done()
    if (priv->sync_command_response)   <- sees the caller's buffer

CPU2 (a sync caller such as mlme_reset_request_sync())
ca8210_spi_exchange()
  wait_for_completion_interruptible_timeout() returns 0 or -ERESTARTSYS
  priv->sync_command_response = NULL;
  returns, and the on-stack response buffer goes out of scope

CPU1
    memcpy(priv->sync_command_response, buf, len);

That would write up to sizeof(struct mac_message) bytes supplied by the
device into a stack frame that the caller no longer owns.

Also, ca8210_spi_exchange() callers are not serialized by any mutex. Can
two concurrent synchronous callers overwrite each other's
sync_command_response and sync_exchange_complete? If so, one caller's
reply could land in the other caller's buffer.

[Severity: High]
This isn't a bug introduced by this patch, but can ca8210_rx_done() sleep
in a context where sleeping is not allowed?

The only caller of ca8210_rx_done() is ca8210_spi_transfer_complete().
ca8210_spi_transfer() installs that function as the spi_async() completion
callback:

	cas_ctl->msg.complete = ca8210_spi_transfer_complete;
	...
	status = spi_async(spi, &cas_ctl->msg);

The spi_async() kernel-doc in drivers/spi/spi.c says:

 * The completion callback is invoked in a context which can't sleep.

When ca8210_rx_done() receives SPI_MCPS_DATA_CONFIRM with
IEEE802154_TRANSACTION_OVERFLOW, it does this:

		if (buf[3] == IEEE802154_TRANSACTION_OVERFLOW) {
			...
			msleep(2000);
			...
			mlme_reset_wpc = kmalloc_obj(*mlme_reset_wpc);

Some controllers finalize messages from IRQ or tasklet context. Would this
trigger "scheduling while atomic" there?

Other controllers finalize from the message pump kthread. There, it looks
like this would stall the SPI bus for two seconds.

[Severity: Low]
This is a pre-existing issue, but ca8210_spi_transfer_complete() dumps the
received buffer using the device's length byte without checking it. That
happens before ca8210_rx_done() reaches the new check:

	if (duplex_rx) {
		dev_dbg(&priv->spi->dev, "READ CMD DURING TX\n");
		for (i = 0; i < cas_ctl->tx_in_buf[1] + 2; i++)
			dev_dbg(
				&priv->spi->dev,
				"%#03x\n",
				cas_ctl->tx_in_buf[i]
			);
		ca8210_rx_done(cas_ctl);
	}

If tx_in_buf[1] == 0xff, does i reach 256 and read tx_in_buf[256]? That is
one byte past the end of the CA8210_SPI_BUF_SIZE array.

This only happens when the dev_dbg() call site is enabled. The loop also
prints bytes beyond the sizeof(struct mac_message) bytes that the transfer
actually received.

Should this loop be limited to sizeof(struct mac_message) as well?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071914.421586-1-benquike%40gmail.com

Re: [PATCH net v3 2/3] ieee802154: ca8210: prevent stack buffer overflow in hwme_get_request_sync()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message overstates how reachable the bug is, and the
  stable backport is not justified.
- [Low] The patch changes the contract of hwme_get_request_sync(), but
  its kernel-doc was not updated.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the sync
  response pointer can be written to after its stack buffer is gone…
- [Medium] This problem wasn't introduced by this patch, but
  hwme_get_request_sync() uses a stack 'struct mac_message response'…

--

ieee802154: ca8210: prevent stack buffer overflow in hwme_get_request_sync()

This commit adds an upper-bound check to hwme_get_request_sync(). If a
HWME-GET confirm reports an hw_attribute_length larger than the
caller-supplied *hw_attribute_length, the function now returns
IEEE802154_SYSTEM_ERROR instead of copying past the end of the destination
buffer. It also sets lenvar to 1 in ca8210_get_ed(), so the size of the
1-byte level buffer is passed in.

> In ca8210_get_ed(), an uninitialized u8 lenvar and a pointer to a 1-byte
> stack buffer (u8 *level) are passed to hwme_get_request_sync(), which
> unconditionally copies response.pdata.hwme_get_cnf.hw_attribute_length
> bytes into hw_attribute_value without checking the caller's destination
> buffer capacity, overflowing level on the stack when hw_attribute_length
> exceeds 1:

[ ... ]

> Tested in QEMU with KASAN enabled by passing an oversized
> hw_attribute_length response into ca8210_get_ed().
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Cc: stable@vger.kernel.org

[Severity: Low]
Can any in-tree code path reach this overflow?

ca8210_get_ed() is the only caller of hwme_get_request_sync(). The only
reference to ca8210_get_ed() is the ops table entry:

    .ed = ca8210_get_ed,

In net/mac802154, the only use of ops->ed seems to be the NULL check in
ieee802154_alloc_hw():

    if (WARN_ON(!ops || !(ops->xmit_async || ops->xmit_sync) || !ops->ed ||

Energy detection scans are also rejected in two places.
nl802154_trigger_scan() accepts only NL802154_SCAN_ACTIVE and
NL802154_SCAN_PASSIVE, and mac802154_trigger_scan_locked() has:

    if (request->type != NL802154_SCAN_PASSIVE &&
        request->type != NL802154_SCAN_ACTIVE)
        return -EOPNOTSUPP;

The splat shows init/1 calling ca8210_get_ed() directly. Together with the
testing note quoted above, this looks like it came from a test harness
rather than a path that exists in the tree.

The code change itself looks correct. Could the commit message call this
hardening of a callback that has no in-tree caller today? And could the
Cc: stable tag either be dropped or be given a justification?

> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index a990a0f347ad8..8aa7ffeb22d78 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -1677,6 +1677,9 @@ static u8 hwme_get_request_sync(
>  		return IEEE802154_SYSTEM_ERROR;
>  
>  	if (response.pdata.hwme_get_cnf.status == IEEE802154_SUCCESS) {
> +		if (response.pdata.hwme_get_cnf.hw_attribute_length >
> +		    *hw_attribute_length)
> +			return IEEE802154_SYSTEM_ERROR;

[Severity: Low]
With this check, *hw_attribute_length is now read as the capacity of the
hw_attribute_value buffer, which makes it an in/out parameter. Should the
kernel-doc for hwme_get_request_sync() be updated as well? It still says:

 * @hw_attribute_length: Attribute length
 * @hw_attribute_value:  Pointer to Attribute Value

It doesn't say that callers must set the buffer size before the call. It
also doesn't mention that a successful confirm with a length that is too
large now returns IEEE802154_SYSTEM_ERROR.

A future caller that follows the kernel-doc could make the same
uninitialized lenvar mistake this patch fixes.

>  		*hw_attribute_length =
>  			response.pdata.hwme_get_cnf.hw_attribute_length;
>  		memcpy(

[Severity: Medium]
This is a pre-existing issue, but can a short HWME_GET_CONFIRM make this
function return success with stale data?

response is an uninitialized struct mac_message on the stack in
hwme_get_request_sync(). ca8210_rx_done() copies into it only the length
the device advertises:

    len = buf[1] + 2;
    ...
    memcpy(priv->sync_command_response, buf, len);

If the confirm is truncated, hw_attribute_length and hw_attribute_value[0]
still hold whatever was left on the stack.

A length of 0 passes the new check. Nothing is copied, and SUCCESS is
returned. ca8210_get_ed() then returns 0 without ever writing *level,
because it doesn't check lenvar after the call.

A length of 1 in a truncated frame would copy one uninitialized stack byte
into *level and still report success.

Would it make sense to:

- check that response.length covers the confirm fields and the claimed
  value,
- require lenvar == 1 in ca8210_get_ed(), and
- zero-initialize response?

As noted above, mac802154 doesn't currently call ops->ed.

> @@ -2027,7 +2030,7 @@ static int ca8210_xmit_async(struct ieee802154_hw *hw, struct sk_buff *skb)
>   */
>  static int ca8210_get_ed(struct ieee802154_hw *hw, u8 *level)
>  {
> -	u8 lenvar;
> +	u8 lenvar = 1;
>  	struct ca8210_priv *priv = hw->priv;
>  
>  	return link_to_linux_err(

[Severity: High]
This is a pre-existing issue, but can ca8210_rx_done() write into the
response buffer in hwme_get_request_sync() after that function has
returned?

ca8210_spi_exchange() publishes the caller's stack buffer:

    priv->sync_command_response = response;

ca8210_rx_done(), which runs from the SPI completion path, then does:

    if (priv->sync_command_response) {
        memcpy(priv->sync_command_response, buf, len);
        complete(&priv->sync_exchange_complete);

It holds no lock and doesn't use READ_ONCE().

The wait on sync_exchange_complete can be interrupted by a signal
(-ERESTARTSYS) or can time out after CA8210_SYNC_TIMEOUT. In either case,
ca8210_spi_exchange() does:

cleanup:
    priv->sync_command_response = NULL;
    return status;

Nothing waits for an rx_done() that is already running:

CPU1                                  CPU2
ca8210_spi_exchange()                 ca8210_rx_done()
                                        if (priv->sync_command_response)
  wait interrupted or timed out
  priv->sync_command_response = NULL
  return
hwme_get_request_sync() returns
                                        memcpy(priv->sync_command_response,
                                               buf, len);

In that case the memcpy() writes up to sizeof(struct mac_message) bytes
into a stack frame that has already been unwound. If the compiler reloads
the pointer after it has been cleared, the result is a write through a NULL
pointer instead.

The same pattern applies to every *_request_sync() helper, for example
ca8210_set_channel()->mlme_set_request_sync(). Nothing in the driver locks
sync_command_response, and priv->lock is only used in the TX path.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071914.421586-1-benquike%40gmail.com

Re: [PATCH net v3 3/3] ieee802154: ca8210: validate data_ind length upfront in ca8210_skb_rx()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says the patch fixes a slab-out-of-bounds
  read, quotes a KASAN splat from a kmalloc-32 object and adds Cc:…
- [Medium] The new truncation check in ca8210_skb_rx() reads
  priv->promiscuous without a lock at line 1781 to decide whether to…

--

ieee802154: ca8210: validate data_ind length upfront in ca8210_skb_rx()

This patch moves the length checks in ca8210_skb_rx() ahead of the skb
allocation. It adds checks that len covers the fixed 30-byte header, the
msdu payload, and the security header when one is present. It also
zero-initializes hdr.

> In ca8210_skb_rx(), data_ind is read at offsets 22 (msdulen), 23
> (mpdulinkquality), 29 + msdulen (hdr.sec.level), 30..39 + msdulen
> (security header), and 29 .. 29 + msdulen (payload) without verifying
> that the received SPI frame length len covers those offsets, causing an
> out-of-bounds read when msdulen exceeds len - 30:
>
>   BUG: KASAN: slab-out-of-bounds in ca8210_skb_rx.constprop.0.isra.0+0x137/0x160
>   Read of size 64 at addr ffff888006453ddd by task init/1

[ ... ]

>   The buggy address belongs to the object at ffff888006453dc0
>    which belongs to the cache kmalloc-32 of size 32

[ ... ]

> Tested in QEMU with KASAN enabled by passing a short data_ind buffer with
> msdulen = 64 and len = 30 into ca8210_skb_rx().

[Severity: Medium]
Can this slab-out-of-bounds read happen on a real driver path? As far as
I can tell, ca8210_skb_rx() has only one caller chain,
ca8210_spi_transfer_complete()->ca8210_rx_done()->ca8210_net_rx(), and
data_ind always points into the fixed-size tx_in_buf:

drivers/net/ieee802154/ca8210.c:
#define CA8210_SPI_BUF_SIZE 256

struct cas_control {
    ...
	u8 tx_in_buf[CA8210_SPI_BUF_SIZE];
};

ca8210_rx_done() {
    ...
	buf = cas_ctl->tx_in_buf;
    ...
	ca8210_net_rx(priv->hw, buf, len);
}

ca8210_net_rx() {
    ...
		return ca8210_skb_rx(hw, len - 2, command + 2);
}

The existing check already caps msdulen at IEEE802154_MTU (127). That
makes the largest read data_ind[39 + 127], which is tx_in_buf[168]. The
payload copy of data_ind[29..155] also stays well inside the 256-byte
array, both before and after this patch.

The kmalloc-32 object in the splat seems to come from the test harness,
which passed a short buffer directly into ca8210_skb_rx(). The trace
still shows ca8210_net_rx() as the caller, though, so it reads like a
live heap over-read.

The real effect looks like a data-integrity problem, not a memory-safety
one. Bytes past the length the device declared get delivered as payload
and security header fields. Those bytes are the rest of the MISO transfer
or the SPI_IDLE fill from ca8210_spi_transfer(), and this can only happen
with a buggy or malicious SPI peripheral.

Could the commit message describe it that way instead of presenting a
KASAN slab-out-of-bounds report? The Cc: stable tag was added on the
basis of that report.

> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index 8aa7ffeb22d78..ab245add9ff5c 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c

[ ... ]

> @@ -1778,9 +1774,25 @@ static int ca8210_skb_rx(
>  			&priv->spi->dev,
>  			"received erroneously large msdu length!\n"
>  		);
> -		kfree_skb(skb);
>  		return -EMSGSIZE;
>  	}
> +
> +	if (len < 30 + msdulen ||
> +	    (!priv->promiscuous && data_ind[29 + msdulen] > 0 &&
> +	     len < 29 + msdulen + sizeof(struct secspec))) {
> +		dev_err(&priv->spi->dev,
> +			"received truncated data indication!\n");
> +		return -EMSGSIZE;
> +	}
> +
> +	mpdulinkquality = data_ind[23];
> +
> +	/* Allocate mtu size buffer for every rx packet */
> +	skb = dev_alloc_skb(IEEE802154_MTU + sizeof(hdr));
> +	if (!skb)
> +		return -ENOMEM;
> +
> +	skb_reserve(skb, sizeof(hdr));
>  	dev_dbg(&priv->spi->dev, "skb buffer length = %d\n", msdulen);
>
>  	if (priv->promiscuous)

[Severity: Medium]
Can the two reads of priv->promiscuous in ca8210_skb_rx() disagree? The
new truncation check reads the flag once, and the later "if
(priv->promiscuous) goto copy_payload;" reads it again. The
dev_alloc_skb() call in between forces a reload.

priv->promiscuous is a plain bool with no locking. It is written from
process context by ca8210_set_promiscuous_mode(), which mac802154 calls
through drv_set_promiscuous_mode():

ca8210_set_promiscuous_mode() {
    ...
	} else {
		priv->promiscuous = on;
	}
}

ca8210_reset_send() also sets it to false. The RX path runs from SPI
completion context, and ca8210_net_rx() drops priv->lock before it calls
ca8210_skb_rx(). So one possible interleaving is:

RX: ca8210_rx_done()->ca8210_net_rx()->ca8210_skb_rx()
    new check sees priv->promiscuous == true
    secspec length check skipped, len == 30 + msdulen

                        ca8210_set_promiscuous_mode(hw, false)
                            priv->promiscuous = on;   /* false */

RX: dev_alloc_skb()
    if (priv->promiscuous)         /* now false, no goto */
    hdr.sec.level = data_ind[29 + msdulen];     /* > 0 */
    hdr.sec.key_id_mode = data_ind[30 + msdulen];
    memcpy(&hdr.sec.extended_src, &data_ind[31 + msdulen], 8);
    hdr.sec.key_id = data_ind[39 + msdulen];

These reads go up to 10 bytes past the validated len. This is the case
the new check is meant to reject. The bytes are still inside the
256-byte tx_in_buf, so the result is garbage security header fields
rather than a memory-safety fault.

Would it make sense to read the flag once into a local, for example
"bool promisc = READ_ONCE(priv->promiscuous);", and use it in both
places?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930071914.421586-1-benquike%40gmail.com

Re: [PATCH net v3 3/3] ieee802154: ca8210: validate data_ind length upfront in ca8210_skb_rx()

[Miquel Raynal]

Hi Hui Peng,

>  
>  	msdulen = data_ind[22]; /* msdu_length */
>  	if (msdulen > IEEE802154_MTU) {
> @@ -1778,9 +1774,25 @@ static int ca8210_skb_rx(
>  			&priv->spi->dev,
>  			"received erroneously large msdu length!\n"
>  		);
> -		kfree_skb(skb);
>  		return -EMSGSIZE;
>  	}
> +
> +	if (len < 30 + msdulen ||
> +	    (!priv->promiscuous && data_ind[29 + msdulen] > 0 &&
> +	     len < 29 + msdulen + sizeof(struct secspec))) {
> +		dev_err(&priv->spi->dev,
> +			"received truncated data indication!\n");
> +		return -EMSGSIZE;
> +	}

I'm sorry, but this is pure AI illusion of security. I'm pretty sure
this check is tailored to match your very specific need but has no
meaning except just covering the very specific case initially
discovered. I don't know what to propose, but I can't send a Reviewed-by
for that.

Miquèl

Re: [PATCH net v3 1/3] ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

[Miquel Raynal]

> Check len > sizeof(struct mac_message) instead of
> len > CA8210_SPI_BUF_SIZE in ca8210_rx_done() so that any packet exceeding
> sizeof(struct mac_message) is logged as erroneously long via dev_crit()
> and dropped before copying into priv->sync_command_response or passing it
> to ca8210_net_rx().
>
> Tested in QEMU with KASAN enabled by passing a 256-byte SPI_SYN response
> into ca8210_rx_done().
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Re: [PATCH net v3 2/3] ieee802154: ca8210: prevent stack buffer overflow in hwme_get_request_sync()

[Miquel Raynal]

> Initialize lenvar = 1 in ca8210_get_ed() and return
> IEEE802154_SYSTEM_ERROR in hwme_get_request_sync() if
> response.pdata.hwme_get_cnf.hw_attribute_length exceeds
> *hw_attribute_length.

The relationship is not obvious to me.

> Tested in QEMU with KASAN enabled by passing an oversized
> hw_attribute_length response into ca8210_get_ed().
>
> Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v3:
> - No changes.
>
> Changes in v2:
> - Split out as patch 2/3.
> - Replaced the temporary stack buffer in ca8210_get_ed() with lenvar = 1
>   and an upper-bound check against *hw_attribute_length in
>   hwme_get_request_sync() as requested by Miquel Raynal.
>
>  drivers/net/ieee802154/ca8210.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index a990a0f..8aa7ffe 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -1677,6 +1677,9 @@ static u8 hwme_get_request_sync(
>  		return IEEE802154_SYSTEM_ERROR;
>  
>  	if (response.pdata.hwme_get_cnf.status == IEEE802154_SUCCESS) {
> +		if (response.pdata.hwme_get_cnf.hw_attribute_length >
> +		    *hw_attribute_length)
> +			return IEEE802154_SYSTEM_ERROR;

Why is hw_attribute_length use here before being set (or re-set) just
below?

The organization of this patch is weird.

Miquèl

[PATCH net v3 1/3] ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

[Hui Peng]

In ca8210_spi_transfer(), each SPI transfer reads
sizeof(struct mac_message) bytes into cas_ctl->tx_in_buf:

  cas_ctl->transfer.len = sizeof(struct mac_message);

However, ca8210_rx_done() only checks the received packet length
len = buf[1] + 2 against CA8210_SPI_BUF_SIZE (256). When buf[0] & SPI_SYN
is set and priv->sync_command_response is non-NULL, memcpy() copies up to
256 bytes into priv->sync_command_response, which points to a
struct mac_message object on the synchronous caller's stack, overflowing
the stack buffer:

  BUG: KASAN: stack-out-of-bounds in ca8210_rx_done+0x117/0x6c0
  Write of size 256 at addr ffff8881009e7c40 by task swapper/0/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   kasan_check_range+0x125/0x200
   __asan_memcpy+0x3c/0x60
   ca8210_rx_done+0x117/0x6c0
  ...
  This frame has 1 object:
   [32, 182) 'response'

Check len > sizeof(struct mac_message) instead of
len > CA8210_SPI_BUF_SIZE in ca8210_rx_done() so that any packet exceeding
sizeof(struct mac_message) is logged as erroneously long via dev_crit()
and dropped before copying into priv->sync_command_response or passing it
to ca8210_net_rx().

Tested in QEMU with KASAN enabled by passing a 256-byte SPI_SYN response
into ca8210_rx_done().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v3:
- Check len > sizeof(struct mac_message) at the top of ca8210_rx_done()
  where dev_crit() logs the error and drops the packet instead of silently
  truncating memcpy() with min_t(), addressing David Laight's feedback.

Changes in v2:
- Split the ca8210 fixes into three single-issue patches (1/3..3/3).

 drivers/net/ieee802154/ca8210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 01af4f9..a990a0f 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -686,7 +686,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
 
 	buf = cas_ctl->tx_in_buf;
 	len = buf[1] + 2;
-	if (len > CA8210_SPI_BUF_SIZE) {
+	if (len > sizeof(struct mac_message)) {
 		dev_crit(
 			&priv->spi->dev,
 			"Received packet len (%u) erroneously long\n",
-- 
2.49.0

[PATCH net v3 1/3] ieee802154: ca8210: prevent stack buffer overflow in ca8210_rx_done()

[Hui Peng]

In ca8210_spi_transfer(), each SPI transfer reads
sizeof(struct mac_message) bytes into cas_ctl->tx_in_buf:

  cas_ctl->transfer.len = sizeof(struct mac_message);

However, ca8210_rx_done() only checks the received packet length
len = buf[1] + 2 against CA8210_SPI_BUF_SIZE (256). When buf[0] & SPI_SYN
is set and priv->sync_command_response is non-NULL, memcpy() copies up to
256 bytes into priv->sync_command_response, which points to a
struct mac_message object on the synchronous caller's stack, overflowing
the stack buffer:

  BUG: KASAN: stack-out-of-bounds in ca8210_rx_done+0x117/0x6c0
  Write of size 256 at addr ffff8881009e7c40 by task swapper/0/1
  Call Trace:
   <TASK>
   dump_stack_lvl+0x70/0xa0
   print_report+0x153/0x4c6
   kasan_report+0xf1/0x120
   kasan_check_range+0x125/0x200
   __asan_memcpy+0x3c/0x60
   ca8210_rx_done+0x117/0x6c0
  ...
  This frame has 1 object:
   [32, 182) 'response'

Check len > sizeof(struct mac_message) instead of
len > CA8210_SPI_BUF_SIZE in ca8210_rx_done() so that any packet exceeding
sizeof(struct mac_message) is logged as erroneously long via dev_crit()
and dropped before copying into priv->sync_command_response or passing it
to ca8210_net_rx().

Tested in QEMU with KASAN enabled by passing a 256-byte SPI_SYN response
into ca8210_rx_done().

Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v3:
- Check len > sizeof(struct mac_message) at the top of ca8210_rx_done()
  where dev_crit() logs the error and drops the packet instead of silently
  truncating memcpy() with min_t(), addressing David Laight's feedback.

Changes in v2:
- Split the ca8210 fixes into three single-issue patches (1/3..3/3).

 drivers/net/ieee802154/ca8210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index 01af4f9..a990a0f 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -686,7 +686,7 @@ static void ca8210_rx_done(struct cas_control *cas_ctl)
 
 	buf = cas_ctl->tx_in_buf;
 	len = buf[1] + 2;
-	if (len > CA8210_SPI_BUF_SIZE) {
+	if (len > sizeof(struct mac_message)) {
 		dev_crit(
 			&priv->spi->dev,
 			"Received packet len (%u) erroneously long\n",
-- 
2.49.0