Re: [PATCH] Bluetooth: btintel_pcie: validate RX packet length against buffer size

[Paul Menzel]

Dear Junrui,


Thank you for your patch. It be great if you configured your name in the 
author line – currently it only contains the address:

    From: moonafterrain@xxxxxxxxxxx

No idea, why b4 is not doing it.

Am 17.03.26 um 07:04 schrieb moonafterrain@xxxxxxxxxxx:
> btintel_pcie_submit_rx_work() reads packet_len from an rfh_hdr in
> DMA-coherent memory and uses it as the length for skb_put_data() without
> upper bound validation. Since packet_len is a 16-bit field (0-65535) but
> each RX DMA buffer is only BTINTEL_PCIE_BUFFER_SIZE (4096) bytes, a
> malicious or malfunctioning firmware could set a large packet_len,
> causing an out-of-bounds read beyond the buffer into adjacent kernel
> heap memory.
>
> Add a check that packet_len does not exceed the available payload space
> alongside the existing zero-length check.

Do you have a reproducer or test case for this issue?

> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> Reported-by: Yuhao Jiang <danisjiang@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>
> ---
>   drivers/bluetooth/btintel_pcie.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 37b744e35bc4..9dd02e8af2a0 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -1360,7 +1360,8 @@ static int btintel_pcie_submit_rx_work(struct btintel_pcie_data *data, u8 status
>   	rfh_hdr = buf;
>   
>   	len = rfh_hdr->packet_len;
> -	if (len <= 0) {
> +	if (len <= 0 ||
> +	    len > BTINTEL_PCIE_BUFFER_SIZE - sizeof(*rfh_hdr)) {
>   		ret = -EINVAL;

As this seems a broken or malicious firmware, no idea, if it’d make 
sense to log it.

>   		goto resubmit;
>   	}

The diff looks good:

Reviewed-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>


Kind regards,

Paul