[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(
-- 
2.49.0

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

[David Laight]

On Tue, 22 Sep 2026 09:30:24 +0000
Hui Peng <benquike@gmail.com> wrote:

> 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.

This function is silly.
There is exactly one caller, the only valid length seems to 1 (is zero valid?),
not much point using memcpy() either.

David

> 
> 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(

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] hwme_get_request_sync() (drivers/net/ieee802154/ca8210.c:1655) is
  changed so that *hw_attribute_length becomes a mandatory *input*: line…

Pre-existing issues:
- [Medium] hwme_get_request_sync() validates the device-reported
  hw_attribute_length only from above; it never checks that the confirm…

--

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

The patch turns the hw_attribute_length argument of
hwme_get_request_sync() into an in/out parameter: the caller-supplied
value is now used as the capacity of the destination buffer, and an
over-long HWME-GET.confirm is rejected with IEEE802154_SYSTEM_ERROR.
The single caller ca8210_get_ed() initialises lenvar to 1 to match the
one-byte level buffer.

> 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]
This isn't a bug, but should the kernel-doc above hwme_get_request_sync()
be updated to describe the new contract?  The block still reads:

  /**
   * hwme_get_request_sync() - HWME_GET_request/confirm according to API Spec
   * @hw_attribute:        Attribute Number
   * @hw_attribute_length: Attribute length
   * @hw_attribute_value:  Pointer to Attribute Value
   * @device_ref:          Nondescript pointer to target device
   *
   * Return: 802.15.4 status code of HWME-GET.confirm
   */

The @hw_attribute_length wording is word for word the same as the one used
for hwme_set_request_sync(), where the parameter really is a plain input
value, and the Return: line no longer covers the new
IEEE802154_SYSTEM_ERROR case for an over-long reply.

A second call site that follows the current wording and passes the expected
attribute length (or leaves the variable uninitialised, as ca8210_get_ed()
did before this patch) would either reintroduce the overflow being fixed
here or make every valid reply fail.  Would it help to document that the
caller must pass sizeof(destination buffer) on entry and receives the number
of bytes written on return?

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

[Severity: Medium]
This is a pre-existing issue and not introduced by this patch, but since the
new check sits exactly where a length test belongs: should there also be a
lower-bound or consistency check here?

Only over-long lengths are rejected, so a confirm with
status == IEEE802154_SUCCESS and hw_attribute_length == 0 passes, copies
zero bytes and returns IEEE802154_SUCCESS:

drivers/net/ieee802154/ca8210.c:ca8210_get_ed() {
	u8 lenvar = 1;
	...
	return link_to_linux_err(
		hwme_get_request_sync(HWME_EDVALUE, &lenvar, level, priv->spi)
	);
}

The returned lenvar is ignored, so ca8210_get_ed() reports 0 while *level
was never written, although struct ieee802154_ops.ed in
include/net/mac802154.h expects the callback to store the measured energy.

There is a related case for a short confirm.  'struct mac_message response'
in hwme_get_request_sync() is an uninitialised stack object, and
ca8210_rx_done() only enforces an upper bound before copying:

drivers/net/ieee802154/ca8210.c:ca8210_rx_done() {
	buf = cas_ctl->tx_in_buf;
	len = buf[1] + 2;
	if (len > sizeof(struct mac_message)) {
		...
		goto finish;
	}

	if (buf[0] & SPI_SYN) {
		if (priv->sync_command_response) {
			memcpy(priv->sync_command_response, buf, len);
	...
}

If the device answers SPI_HWME_GET_CONFIRM with a length byte that stops
before the attribute bytes, response.pdata.hwme_get_cnf.hw_attribute_length
and hw_attribute_value keep whatever was on the stack.  When that stale
length happens to be 0 or 1 the new check passes, and a stale stack byte is
copied into *level while success is returned.  Would an exact length test
for the fixed-width HWME_EDVALUE attribute, plus a check that the confirmed
attribute length fits within response.length, close both cases?

In the current tree neither variant is reachable from userspace, since ->ed
has no in-tree caller (net/mac802154/main.c only checks for its presence
with a WARN_ON, and ED scans are refused in net/mac802154/scan.c).

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

[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)
-- 
2.49.0

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 3 potential
issue(s) to consider.

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

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 mischaracterises the defect: it pastes a
  KASAN "slab-out-of-bounds ... kmalloc-32 of size 32" report and…
- [Medium] The patch silently changes 'struct ieee802154_hdr hdr;' to
  'struct ieee802154_hdr hdr = { };' (ca8210.c:1761) without mentioning…
- [Medium] The new consolidated validation in ca8210_skb_rx() gates the
  security-header length check on priv->promiscuous…

Pre-existing issues:
- [Low] ca8210_net_rx() reads command[26] (== data_ind[24], the DSN of an
  MCPS_DATA_INDICATION) and latches it into priv->last_dsn…
- [Low] Concern (reported for completeness, assessed as not actionable):
  hdr is now explicitly zero-initialized and no path assigns hdr.seq,…

--

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

This moves all length and msdulen validation in ca8210_skb_rx() into a
single check at the top of the function, before dev_alloc_skb(), and adds
a new check that the reported SPI frame length covers the payload and the
optional security header.  The hdr declaration is also changed to be
zero-initialized.

A few questions below.

> 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

[Severity: Medium]
Can this slab-out-of-bounds actually be reached from the driver's receive
path?

ca8210_skb_rx() has one caller chain:

  ca8210_spi_transfer_complete() -> ca8210_rx_done() -> ca8210_net_rx()
    -> ca8210_skb_rx(hw, len - 2, command + 2)

and the buffer is always cas_ctl->tx_in_buf, a fixed array embedded in a
kzalloc'd struct cas_control:

drivers/net/ieee802154/ca8210.c:
    struct cas_control {
        ...
        u8 tx_in_buf[CA8210_SPI_BUF_SIZE];
    ...
    };

with CA8210_SPI_BUF_SIZE == 256, not a kmalloc-32 allocation.  Since the
pre-existing check caps msdulen at IEEE802154_MTU (127), the furthest byte
the unpatched code can touch is data_ind[39 + 127] == tx_in_buf[168], and
skb_put_data() copies tx_in_buf[31..157] — all inside the 256-byte array.

The changelog's own note:

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

suggests the splat came from a synthetic direct call on a 32-byte buffer
rather than from the SPI receive path.  On the real path the observable
effect of the missing checks looks like stale SPI buffer bytes (idle fill
0xFF, or residue from a previous exchange) being copied into the skb and
into hdr.sec, which is a data-integrity issue rather than memory
corruption.

Could the commit message describe the actual in-tree consequence, and
could the Fixes:/Cc: stable rationale be restated accordingly so stable
triage isn't misled by the pasted KASAN 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
> @@ -1758,19 +1758,15 @@ static int ca8210_skb_rx(
>  	u8                    *data_ind
>  )
>  {
> -	struct ieee802154_hdr hdr;
> +	struct ieee802154_hdr hdr = { };

[Severity: Medium]
Is this zero-initialization a separate fix that deserves a mention in the
changelog?

ieee802154_hdr_push() unconditionally emits the sequence octet and both
frame control bytes:

net/ieee802154/header_ops.c:ieee802154_hdr_push() {
	...
	buf[pos++] = hdr->seq;
	...
	memcpy(buf, fc, 2);
	memcpy(skb_push(skb, pos), buf, pos);
}

while ca8210_skb_rx() only assigns hdr.fc.type, hdr.fc.security_enabled,
hdr.fc.intra_pan and the two address-mode fields, and never assigns
hdr.seq, hdr.fc.version (outside the security path), hdr.fc.ack_request,
hdr.fc.frame_pending or the reserved bits.  Before this hunk those bytes
were uninitialized stack data pushed into every reconstructed header handed
to mac802154 via ieee802154_rx_irqsafe(); after it they are deterministic
zeros.

The only trace of this is below the --- line ("dropped the unrelated
hdr.seq assignment"), which does not end up in the git log.  Should the
changelog describe this behaviour change, or should the hunk be split into
its own patch?

[Severity: Low]
This isn't a bug introduced by this patch, but with hdr now explicitly
zeroed and no path assigning hdr.seq, every non-promiscuous frame is
reported upstream with sequence number 0, even though the driver does read
the real DSN from the same message in ca8210_net_rx():

	if (command[26] == priv->last_dsn) {

Previously the emitted octet was stack garbage, so this is unchanged in
kind, and the v2 changelog records that wiring up hdr.seq was dropped at
the maintainer's request.  Noting it only so the intent is on record.

>  	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;

[Severity: Low]
This is a pre-existing issue, but the caller parses the same message with
no minimum length at all, so the new "upfront" check is not the first
parse of the frame:

drivers/net/ieee802154/ca8210.c:ca8210_rx_done() {
	buf = cas_ctl->tx_in_buf;
	len = buf[1] + 2;
	if (len > sizeof(struct mac_message)) {
	...
}

drivers/net/ieee802154/ca8210.c:ca8210_net_rx() {
	...
	if (command[26] == priv->last_dsn) {
	...
	priv->last_dsn = command[26];
	spin_unlock_irqrestore(&priv->lock, flags);
	return ca8210_skb_rx(hw, len - 2, command + 2);
}

len is only bounded from above, so a data indication declaring buf[1] < 25
still has command[26] (== data_ind[24]) read and latched into
priv->last_dsn before ca8210_skb_rx() gets to reject it.  The read stays
inside tx_in_buf, but the byte is idle fill or residue, so last_dsn can be
poisoned and a later legitimate frame carrying that DSN is dropped as a
resend.

Would it make sense to also give ca8210_net_rx() a minimum length check
for SPI_MCPS_DATA_INDICATION, so both parsers of this message format agree
on its minimum size?

>  
>  	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;
> +	}

[Severity: Medium]
Can the two reads of priv->promiscuous disagree here?

The security-header length check is gated on !priv->promiscuous above, but
the decision to actually parse the security header is a second, independent
read a few lines later:

	if (priv->promiscuous)
		goto copy_payload;

	/* Populate hdr */
	hdr.sec.level = data_ind[29 + msdulen];
	...
		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];

priv->promiscuous is a plain bool written without any lock, READ_ONCE() or
barrier:

drivers/net/ieee802154/ca8210.c:ca8210_set_promiscuous_mode() {
	...
		priv->promiscuous = on;
}

drivers/net/ieee802154/ca8210.c:ca8210_reset_send() {
	...
	priv->promiscuous = false;
}

and ca8210_net_rx() drops priv->lock before calling ca8210_skb_rx():

	spin_unlock_irqrestore(&priv->lock, flags);
	return ca8210_skb_rx(hw, len - 2, command + 2);

So if the flag reads true at the check and false at the use — for example
a monitor-mode toggle from ca8210_set_promiscuous_mode() landing between
the two — the "len < 29 + msdulen + sizeof(struct secspec)" branch is
never evaluated, yet data_ind[29 + msdulen] through data_ind[39 + msdulen]
are dereferenced.  That reads up to 11 bytes past the end of the frame the
length check validated (stale tx_in_buf contents), producing a fabricated
hdr.sec or an ieee802154_hdr_push() failure and a dropped frame, which is
the exact byte class this patch sets out to stop touching.

Would taking one snapshot, e.g.

	bool promisc = READ_ONCE(priv->promiscuous);

and using it for both the check and the parse decision, or validating the
security-header length unconditionally, close that window?

> +
> +	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)

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

[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 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(

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 reads as if the stack overflow in
  ca8210_get_ed()/hwme_get_request_sync() can happen at runtime.
- [Low] The patch turns `hw_attribute_length` in hwme_get_request_sync()
  into an in/out parameter, but the kernel-doc was not updated.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but it is a real
  memory-safety race in the same sync-exchange path.

--

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

This patch initializes lenvar to 1 in ca8210_get_ed(). It also makes
hwme_get_request_sync() return IEEE802154_SYSTEM_ERROR when the device
reports an hw_attribute_length larger than the caller's buffer. The goal
is to stop a stack overflow of the 1-byte level buffer in ca8210_get_ed().

> 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

[ ... ]

> 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 this overflow actually be reached at runtime? The commit message reads
as if it describes live stack corruption. It shows a splat from task init/1
and carries a stable tag.

hwme_get_request_sync() is static. Its only caller is ca8210_get_ed(), and
ca8210_get_ed() is only referenced as the .ed callback in ca8210_phy_ops:

	.ed = ca8210_get_ed,

As far as I can tell, mac802154 never calls ops->ed. The only reference is
the NULL check in ieee802154_alloc_hw():

net/mac802154/main.c:ieee802154_alloc_hw() {
	...
	if (WARN_ON(!ops || !(ops->xmit_async || ops->xmit_sync) || !ops->ed ||
	...
}

nl802154_trigger_scan() also accepts only NL802154_SCAN_ACTIVE and
NL802154_SCAN_PASSIVE. So an energy-detection scan from userspace can't
get there either.

Even if there were a caller, the device would have to answer
HWME_GET(HWME_EDVALUE) with hw_attribute_length > 1.

Could the commit message say that the .ed op is not currently invoked, and
that a misbehaving CA8210 would be needed? That would help maintainers
choose between net and net-next, and decide whether a stable backport is
needed.

> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index a990a0f347ad..8aa7ffeb22d7 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]
Should the kernel-doc for hwme_get_request_sync() be updated too?

*hw_attribute_length is now read as the size of hw_attribute_value, and
then overwritten with the copied length. That makes it an in/out
parameter. The comment still says:

 * @hw_attribute_length: Attribute length
 ...
 * Return: 802.15.4 status code of HWME-GET.confirm

Before this patch the argument was only written, which is why
ca8210_get_ed() could pass an uninitialized lenvar. A future caller that
follows the kernel-doc and does the same would compare against a random
value, and the overflow would come back.

The only caller meets the new requirement with a bare "u8 lenvar = 1;".
Nothing ties that 1 to sizeof(*level).

The Return: text is now also wrong in one more case. This
IEEE802154_SYSTEM_ERROR comes from the driver, not from the device's
confirm, and link_to_linux_err() maps it to -EPROTO. Two older
SYSTEM_ERROR returns had already made that text inaccurate.

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

[Severity: High]
This isn't a bug introduced by this patch, but there seems to be a race in
the sync exchange path that this length check can't protect against.

priv->sync_command_response is per-device state shared by all sync
callers, and no lock covers it. ca8210_spi_exchange() publishes the
caller's stack buffer, waits interruptibly with a timeout, and then clears
the pointer:

drivers/net/ieee802154/ca8210.c:ca8210_spi_exchange() {
	...
	if ((buf[0] & SPI_SYN) && response) { /* if sync wait for confirm */
		reinit_completion(&priv->sync_exchange_complete);
		priv->sync_command_response = response;
	}
	...
cleanup:
	priv->sync_command_response = NULL;
	return status;
}

ca8210_rx_done() runs from the SPI completion callback
ca8210_spi_transfer_complete(). It tests the pointer and then copies into
it, with no lock:

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);
	...
}

Suppose the waiter gets -ERESTARTSYS or -ETIME after ca8210_rx_done() has
loaded the non-NULL pointer but before the memcpy. Can that memcpy then
write into the stack frame of a caller that has already returned from
hwme_get_request_sync()?

Sync exchanges also don't appear to be serialized.
ca8210_mlme_reset_worker() calls mlme_reset_request_sync() from a
workqueue without rtnl, and ca8210_rx_done() queues it on
TRANSACTION_OVERFLOW. It can run at the same time as rtnl-held
ieee802154_ops callbacks such as set_channel or set_txpower, which also
use the *_request_sync() helpers.

Could two callers overwrite each other's response pointer, or consume each
other's completion? One caller would get the other command's confirm. The
other would either time out or return with its response buffer untouched,
because hwme_get_request_sync() doesn't preset command_id.

Would it make sense to serialize sync exchanges with a mutex? The handoff
of the response buffer would also need to be synchronized against
ca8210_rx_done(). Ideally, the confirm ID would also be checked against
the request that was issued.

[ ... ]

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

[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 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(

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 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