Re: [PATCH v1 1/2] Bluetooth: btintel_pcie: validate packet_len before skb_put_data

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Kiran,


Thank you for your patch.

Am 02.09.26 um 15:18 schrieb Kiran K:
> btintel_pcie_submit_rx_work() reads packet_len from rfh_hdr without
> checking if it exceeds the RX buffer size. An oversized packet_len
> can lead to an out-of-bounds read in skb_put_data().
> 
> Validate packet_len to ensure it is non-zero and does not exceed
> BTINTEL_PCIE_BUFFER_SIZE - sizeof(*rfh_hdr), logging an error when
> invalid.

Can this case be forced somehow? Please document how.

Did a tool find this case or did you experience it during 
testing/review? If a tool, please document it.

> Fixes: c2b636b3f788 ("Bluetooth: btintel_pcie: Add support for PCIe transport")
> Signed-off-by: Kiran K <kiran.k@intel.com>
> ---
>   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 30923eaabed7..1eabb0c8326f 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -1954,7 +1954,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)) {
> +		bt_dev_err(data->hdev, "Invalid packet_len %d", len);

What about negative values for `len` as was checked before?

To have a more useful log, I’d also log the value of 
`BTINTEL_PCIE_BUFFER_SIZE` and `sizeof(*rfh_hdr)`.

>   		ret = -EINVAL;
>   		goto resubmit;
>   	}


Kind regards,

Paul