Re: [PATCH] Bluetooth: btintel: Fix insufficient skb length check in btintel_print_fseq_info()

[Quan Sun <2022090917019@std.uestc.edu.cn>]

Hi,

On 2026/5/15 1:35, Luiz Augusto von Dentz wrote:
> Hi,
> 
> On Thu, May 14, 2026 at 12:49 PM Quan Sun
> <2022090917019@std.uestc.edu.cn> wrote:
>>
>> The length check at the top of btintel_print_fseq_info() verifies
>> that the skb has at least 66 bytes (sizeof(u32) * 16 + 2), but the
>> function actually consumes 74 bytes:
>>
>>    2 calls to skb_pull_data(skb, 1)  =  2 bytes
>>    18 calls to skb_pull_data(skb, 4) = 72 bytes
>>
>> When the firmware returns a packet of exactly 66 bytes, the last two
>> skb_pull_data(skb, 4) calls return NULL, which is then passed directly
>> to get_unaligned_le32(), resulting in a NULL pointer dereference.
>>
>> Fix the length check to account for all 74 bytes actually consumed:
>>    sizeof(u32) * 16 + 2  ->  sizeof(u32) * 18 + 2
>>
>> Fixes: a7ba218a44aa ("Bluetooth: btintel: Print Firmware Sequencer information")
>> Signed-off-by: Quan Sun <2022090917019@std.uestc.edu.cn>
>> ---
>>   drivers/bluetooth/btintel.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>> index dcaaa4ca02b99..114a8beeab92d 100644
>> --- a/drivers/bluetooth/btintel.c
>> +++ b/drivers/bluetooth/btintel.c
>> @@ -3356,7 +3356,7 @@ void btintel_print_fseq_info(struct hci_dev *hdev)
>>                  return;
>>          }
>>
>> -       if (skb->len < (sizeof(u32) * 16 + 2)) {
>> +       if (skb->len < (sizeof(u32) * 18 + 2)) {
> 
> Or we stop doing this manually and the check the return of
> skb_pull_data, that way we garantee we don't use its returns without
> checking if it return NULL, which is the whole point in using
> skb_pull_data otherwise we had just used skb_pull.
> 
>>                  bt_dev_dbg(hdev, "Malformed packet of length %u received",
>>                             skb->len);
>>                  kfree_skb(skb);
>> --
>> 2.43.0
>>
> 
> 

You are right. I will refactor the function to check the return value of 
each skb_pull_data() call to make it more robust.