Hi Cong,
On 4/2/19 7:42 PM, Cong Wang wrote:
> On Mon, Apr 1, 2019 at 11:33 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>>
>> On Mon, Apr 01, 2019 at 11:03:53AM -0700, Cong Wang wrote:
>>> Hi,
>>>
>>> On Sat, Mar 30, 2019 at 2:23 AM Tomas Bortoli <tomasbortoli@xxxxxxxxx> wrote:
>>>>
>>>> Hi Dan,
>>>>
>>>> On 3/30/19 8:25 AM, Dan Carpenter wrote:
>>>>> There is a potential out of bounds if "ev->length" is too high or if the
>>>>> number of reports are too many.
>>>>>
>>>>> Fixes: c215e9397b00 ("Bluetooth: Process extended ADV report event")
>>>>> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>>>> Reviewed-By: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
>>>
>>> I sent a patchset to fix all of this kind of OOB:
>>> https://marc.info/?l=linux-netdev&m=155314874622831&w=2
Reviewed-by: Tomas Bortoli <tomasbortoli@xxxxxxxxx>
All 3 looks good to me, nice work! Overall, it seems that with these 3
all the RX paths in hci_event.c are bound checked.
>>>
>>> Unfortunately I get no response...
>>>
>>> Does any of you mind to look at them?
>>>
>>
>> I don't know the rules... When is it ok say:
>>
>> if (skb->len < sizeof(*ev))
>> return;
>>
>> and when must we say:
>>
>> if (!pskb_may_pull(skb, sizeof(*ev)))
>> return;
>
>
> The rule is simple: pskb_may_pull() always knows better. In bluetooth
> case, they are actually equivalent, as the skb's in bluetooth are linear.
>
>
>>
>> Btw, get rid of all the likely/unlikely() macros. Then the other style
>> comment would be don't move the "ev = (void *)skb->data;" assignments
>> around. It's ok to say:
>
>
> Similarly, pskb_may_pull() may reallocate skb's, although very unlikely
> for bluetooth case (skb's are linear). At least it doesn't harm anything
> we move the skb->data dereference after pskb_may_pull().
>
> I think these likely()/unlikely() are reasonable, ill-formatted packets
> are rare cases, normal packets deserve such a special care. We
> use likely()/unlikely() with pskb_may_pull() in many places in
> networking subsystem, at least.
>
> Anyway, if you don't mind, I can resend my patchset with you Cc'ed.
>
> Thanks.
>
