netdev - Re: bpf filter : support for vlan tag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD6jFUQ3AMYVrFhpObnDxh24o+wQ+hn5y4+H4gVuv8t84v9pZQ@mail.gmail.com>
Date:	Sat, 20 Oct 2012 13:42:34 +0200
From:	Daniel Borkmann <danborkmann@...earbox.net>
To:	Ani Sinha <ani@...stanetworks.com>
Cc:	Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org
Subject: Re: bpf filter : support for vlan tag

On Fri, Oct 19, 2012 at 11:02 PM, Ani Sinha <ani@...stanetworks.com> wrote:
> On Fri, Oct 19, 2012 at 1:53 PM, Daniel Borkmann
> <danborkmann@...earbox.net> wrote:
>> On Fri, Oct 19, 2012 at 8:32 PM, Ani Sinha <ani@...stanetworks.com> wrote:
>>> how about this?
>>>
>>> On Tue, Oct 16, 2012 at 4:28 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
>>>
>>>> @@ -341,6 +342,12 @@ load_b:
>>>>                 case BPF_S_ANC_CPU:
>>>>                         A = raw_smp_processor_id();
>>>>                         continue;
>>>> +               case BPF_S_ANC_VLAN_TAG:
>>>> +                       A = vlan_tx_tag_get(skb);
>>>> +                       continue;
>>>> +               case BPF_S_ANC_VLAN_TAG_PRESENT:
>>>> +                       A = !!vlan_tx_tag_present(skb);
>>>> +                       continue;
>>>>                 case BPF_S_ANC_NLATTR: {
>>>>                         struct nlattr *nla;
>>>
>>>
>>> +               case BPF_S_ANC_VLAN_TAG:
>>> +                       if (!vlan_tx_tag_present(skb)) {
>>> +                               return 0;
>>> +                       }
>>> +                       A = vlan_tx_tag_get(skb);
>>> +                       continue;
>>
>> I didn't look into the code, but I assume that if no vlan is present,
>> then vlan_tx_tag_get might return 0 anyway.
>
> This might not be true all the time. So it's always safe to do this
> check before returning the VLANID and throw some kind of error if the
> vlan ID is not set.

But wasn't this the reason why Eric added BPF_S_ANC_VLAN_TAG_PRESENT ?
Or is your objection performance related?

>  Also, your return is
>> simply wrong, since then after this instruction you leave the *whole*
>> BPF machine ignoring the rest of the filter program to process ...
>
> I had done that because I can see in other parts of that state machine
> that in error condition the code simply stops processing the packet. I
> am not sure how else to handle the error case.

See comment above. In my opinion the other cases are more severe like
divide by zero and the like, but okay ..
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html