Re: [PATCH 3/5] staging: nvec: fix use-after-free in nvec_rx_completed()

Re: [PATCH 3/5] staging: nvec: fix use-after-free in nvec_rx_completed()

[Alexandru Hossu]

On Mon, Apr 13, 2026, Dan Carpenter wrote:
> Add a Fixes tag.
> Delete this comment. (Obvious).
> Move this declaration to the start of the block.
> Otherwise, it looks good.

Thanks for the review. Will send v2 with:
- Fixes: d6bdcf2e1019 ("staging: nvec: Add battery quirk to ignore incomplete responses")
- Comment removed
- Declaration moved to start of block

Alexandru

[PATCH v2 1/3] staging: nvec: fix use-after-free in nvec_rx_completed()

[Alexandru Hossu]

In nvec_rx_completed(), when an incomplete RX transfer is detected,
nvec_msg_free() is called to return the message back to the pool by
clearing its 'used' atomic flag. Immediately after this, the code
accesses nvec->rx->data[0] to check the message type.

Since nvec_msg_free() marks the pool slot as available via atomic_set(),
any concurrent or subsequent call to nvec_msg_alloc() could claim that
same slot and overwrite its data[] array. Reading nvec->rx->data[0] after
freeing the message is therefore a use-after-free.

Fix this by saving the message type byte before calling nvec_msg_free(),
then using the saved value for the battery quirk check.

Fixes: d6bdcf2e1019 ("staging: nvec: Add battery quirk to ignore incomplete responses")
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
v2:
 - Add Fixes tag (Dan Carpenter)
 - Remove obvious comment (Dan Carpenter)
 - Move declaration to start of block (Dan Carpenter)

 drivers/staging/nvec/nvec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 952c5a849a56..2a3499dd4d63 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -494,6 +494,8 @@ static void nvec_tx_completed(struct nvec_chip *nvec)
 static void nvec_rx_completed(struct nvec_chip *nvec)
 {
 	if (nvec->rx->pos != nvec_msg_size(nvec->rx)) {
+		unsigned char msg_type = nvec->rx->data[0];
+
 		dev_err(nvec->dev, "RX incomplete: Expected %u bytes, got %u\n",
 			(uint)nvec_msg_size(nvec->rx),
 			(uint)nvec->rx->pos);
@@ -502,7 +504,7 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 		nvec->state = 0;
 
 		/* Battery quirk - Often incomplete, and likes to crash */
-		if (nvec->rx->data[0] == NVEC_BAT)
+		if (msg_type == NVEC_BAT)
 			complete(&nvec->ec_transfer);
 
 		return;
-- 
2.53.0

[PATCH 3/5] staging: nvec: fix use-after-free in nvec_rx_completed()

[Alexandru Hossu]

In nvec_rx_completed(), when an incomplete RX transfer is detected,
nvec_msg_free() is called to return the message back to the pool by
clearing its 'used' atomic flag. Immediately after this, the code
accesses nvec->rx->data[0] to check the message type.

Since nvec_msg_free() marks the pool slot as available via atomic_set(),
any concurrent or subsequent call to nvec_msg_alloc() could claim that
same slot and overwrite its data[] array. Reading nvec->rx->data[0] after
freeing the message is therefore a use-after-free.

Fix this by saving the message type byte before calling nvec_msg_free(),
then using the saved value for the battery quirk check.

Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/nvec/nvec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 952c5a849a56..75877038847f 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -498,11 +498,14 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
 			(uint)nvec_msg_size(nvec->rx),
 			(uint)nvec->rx->pos);
 
+		/* Save before freeing to avoid use-after-free */
+		unsigned char msg_type = nvec->rx->data[0];
+
 		nvec_msg_free(nvec, nvec->rx);
 		nvec->state = 0;
 
 		/* Battery quirk - Often incomplete, and likes to crash */
-		if (nvec->rx->data[0] == NVEC_BAT)
+		if (msg_type == NVEC_BAT)
 			complete(&nvec->ec_transfer);
 
 		return;
-- 
2.53.0

Re: [PATCH 3/5] staging: nvec: fix use-after-free in nvec_rx_completed()

[Dan Carpenter]

On Sun, Apr 12, 2026 at 10:51:16PM +0200, Alexandru Hossu wrote:
> In nvec_rx_completed(), when an incomplete RX transfer is detected,
> nvec_msg_free() is called to return the message back to the pool by
> clearing its 'used' atomic flag. Immediately after this, the code
> accesses nvec->rx->data[0] to check the message type.
> 
> Since nvec_msg_free() marks the pool slot as available via atomic_set(),
> any concurrent or subsequent call to nvec_msg_alloc() could claim that
> same slot and overwrite its data[] array. Reading nvec->rx->data[0] after
> freeing the message is therefore a use-after-free.
> 
> Fix this by saving the message type byte before calling nvec_msg_free(),
> then using the saved value for the battery quirk check.
> 
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>

Add a Fixes tag.

> ---
>  drivers/staging/nvec/nvec.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
> index 952c5a849a56..75877038847f 100644
> --- a/drivers/staging/nvec/nvec.c
> +++ b/drivers/staging/nvec/nvec.c
> @@ -498,11 +498,14 @@ static void nvec_rx_completed(struct nvec_chip *nvec)
>  			(uint)nvec_msg_size(nvec->rx),
>  			(uint)nvec->rx->pos);
>  
> +		/* Save before freeing to avoid use-after-free */

Delete this comment.  (Obvious).

> +		unsigned char msg_type = nvec->rx->data[0];

Move this declaration to the start of the block.

Otherwise, it looks good.

regards,
dan carpenter

> +
>  		nvec_msg_free(nvec, nvec->rx);
>  		nvec->state = 0;
>  
>  		/* Battery quirk - Often incomplete, and likes to crash */
> -		if (nvec->rx->data[0] == NVEC_BAT)
> +		if (msg_type == NVEC_BAT)
>  			complete(&nvec->ec_transfer);
>  
>  		return;