[PATCH] soc: qcom: apr: Reject packets with header size larger than packet size

[PATCH] soc: qcom: apr: Reject packets with header size larger than packet size

[Krzysztof Kozlowski]

apr_do_rx_callback() and gpr_do_rx_callback() validate the header size
and the packet size separately, but never check that the header fits
within the packet.  Both sizes are u16, so with a malformed packet where
the header size exceeds the packet size, the subtraction computing
resp.payload_size gets promoted to int and becomes negative.

Further check "resp.payload_size > 0" would fail, leading to
resp.payload left uninitialized (stack value).  Reject such packets as
having a wrong packet size, although unlikely that firmware would
produce such packets.

Fixes: 6adba21eb434 ("soc: qcom: Add APR bus driver")
Fixes: ec1471a898cc ("soc: qcom: apr: Add GPR support")
Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-82d40745-1593-4c20-8432-d86099836525
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 drivers/soc/qcom/apr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c
index 2d5645ff9919..6f29b57aaecc 100644
--- a/drivers/soc/qcom/apr.c
+++ b/drivers/soc/qcom/apr.c
@@ -210,7 +210,8 @@ static int apr_do_rx_callback(struct packet_router *apr, struct apr_rx_buf *abuf
 		return -EINVAL;
 	}
 
-	if (hdr->pkt_size < APR_HDR_SIZE || hdr->pkt_size != len) {
+	if (hdr->pkt_size < APR_HDR_SIZE || hdr->pkt_size != len ||
+	    hdr->pkt_size < hdr_size) {
 		dev_err(apr->dev, "APR: Wrong packet size\n");
 		return -EINVAL;
 	}
@@ -280,7 +281,8 @@ static int gpr_do_rx_callback(struct packet_router *gpr, struct apr_rx_buf *abuf
 		return -EINVAL;
 	}
 
-	if (hdr->pkt_size < GPR_PKT_HEADER_BYTE_SIZE || hdr->pkt_size != len) {
+	if (hdr->pkt_size < GPR_PKT_HEADER_BYTE_SIZE || hdr->pkt_size != len ||
+	    hdr->pkt_size < hdr_size * 4) {
 		dev_err(gpr->dev, "GPR: Wrong packet size\n");
 		return -EINVAL;
 	}
-- 
2.53.0

Re: [PATCH] soc: qcom: apr: Reject packets with header size larger than packet size

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] soc: qcom: apr: uninitialized resp.payload when pkt_size equals hdr_size
--

commit 2669ea074a5a71b19b685f8644fc4fa968102cc7
Author: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

soc: qcom: apr: Reject packets with header size larger than packet size

This commit adds checks in apr_do_rx_callback() and gpr_do_rx_callback() to
reject incoming APR and GPR packets whose header size exceeds the packet size,
preventing a negative resp.payload_size and an uninitialized resp.payload
stack pointer.

> diff --git a/drivers/soc/qcom/apr.c b/drivers/soc/qcom/apr.c
> index 2d5645ff99197..6f29b57aaecce 100644
> --- a/drivers/soc/qcom/apr.c
> +++ b/drivers/soc/qcom/apr.c
> @@ -210,7 +210,8 @@ static int apr_do_rx_callback(struct packet_router *apr, struct apr_rx_buf *abuf
>  		return -EINVAL;
>  	}
>  
> -	if (hdr->pkt_size < APR_HDR_SIZE || hdr->pkt_size != len) {
> +	if (hdr->pkt_size < APR_HDR_SIZE || hdr->pkt_size != len ||
> +	    hdr->pkt_size < hdr_size) {
>  		dev_err(apr->dev, "APR: Wrong packet size\n");
>  		return -EINVAL;
>  	}
> @@ -280,7 +281,8 @@ static int gpr_do_rx_callback(struct packet_router *gpr, struct apr_rx_buf *abuf
>  		return -EINVAL;
>  	}
>  
> -	if (hdr->pkt_size < GPR_PKT_HEADER_BYTE_SIZE || hdr->pkt_size != len) {
> +	if (hdr->pkt_size < GPR_PKT_HEADER_BYTE_SIZE || hdr->pkt_size != len ||
> +	    hdr->pkt_size < hdr_size * 4) {
>  		dev_err(gpr->dev, "GPR: Wrong packet size\n");
>  		return -EINVAL;
>  	}

[Severity: High]
What happens when a packet with optional header words has hdr->pkt_size equal
to hdr_size (in apr_do_rx_callback()) or hdr_size * 4 (in
gpr_do_rx_callback())?

In apr_callback(), packets with len > APR_HDR_SIZE are accepted, so a packet
with optional header words and no payload (for example, len == 24 and a
24-byte header) passes both apr_callback() and these new < checks.

In both apr_do_rx_callback() and gpr_do_rx_callback(), resp is declared on the
stack without initialization, and resp.payload_size evaluates to 0:

	resp.hdr = *hdr;
	resp.payload_size = hdr->pkt_size - hdr_size;

	if (resp.payload_size > 0)
		resp.payload = buf + hdr_size;

	adrv->callback(adev, &resp);

Because resp.payload is only assigned when resp.payload_size > 0, resp.payload
is still left uninitialized on the stack and passed to the service callbacks
(such as q6core_callback(), q6asm_srvc_callback(), graph_callback(), and
apm_callback()), which dereference data->payload without checking
data->payload_size.

Should these checks use <= (hdr->pkt_size <= hdr_size and
hdr->pkt_size <= hdr_size * 4, matching len <= APR_HDR_SIZE in
apr_callback()), or should resp.payload be initialized to NULL if zero-length
payloads are valid?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101175407.110780-2-krzysztof.kozlowski@oss.qualcomm.com?part=1

Re: [PATCH] soc: qcom: apr: Reject packets with header size larger than packet size

[Krzysztof Kozlowski]

On 01/11/2026 18:54, Krzysztof Kozlowski wrote:
> apr_do_rx_callback() and gpr_do_rx_callback() validate the header size
> and the packet size separately, but never check that the header fits
> within the packet.  Both sizes are u16, so with a malformed packet where
> the header size exceeds the packet size, the subtraction computing
> resp.payload_size gets promoted to int and becomes negative.
> 
> Further check "resp.payload_size > 0" would fail, leading to
> resp.payload left uninitialized (stack value).  Reject such packets as
> having a wrong packet size, although unlikely that firmware would
> produce such packets.
> 
> Fixes: 6adba21eb434 ("soc: qcom: Add APR bus driver")
> Fixes: ec1471a898cc ("soc: qcom: apr: Add GPR support")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/bug/linux-82d40745-1593-4c20-8432-d86099836525
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---

Valid Sashiko review.

Best regards,
Krzysztof