netdev - Re: [PATCH net-next] packet mmap : allow the user to choose tx data offset.

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] [day] [month] [year] [list]

Date:	Sat, 20 Oct 2012 19:35:27 +0200
From:	Daniel Borkmann <danborkmann@...earbox.net>
To:	pchavent <Paul.Chavent@...ra.fr>
Cc:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] packet mmap : allow the user to choose tx data offset.

On Sat, Oct 20, 2012 at 7:13 PM, pchavent <Paul.Chavent@...ra.fr> wrote:
> Hi Daniel
>
>
>>>>> +                       switch (po->tp_version) {
>>>>> +                       case TPACKET_V2:
>>>>> +                               off = ph.h2->tp_mac;
>>>>> +                               break;
>>>>> +                       default:
>>>>> +                               off = ph.h1->tp_mac;
>>>>
>>>>
>>>>
>>>> TPACKET_V1 as default is wrong since there's also TPACKET_V3. What
>>>> about TPACKET_V3 in general in your patch? You simply ignore it.
>>>
>>>
>>>
>>> I have reproduced the pattern found some lines above (1868-1875)
>>>
>>> switch (po->tp_version) {
>>> case TPACKET_V2:
>>>         tp_len = ph.h2->tp_len;
>>>         break;
>>> default:
>>>         tp_len = ph.h1->tp_len;
>>>         break;
>>> }
>>>
>>> Do you suggest me to add a case TPACKET_V3 and get tp_len from
>>> ph.h3->tp_len
>>> ?
>>
>>
>> If I'm not mistaken, with your current patch you can do the
>> setsockopt(2) call on a tpacket v3 socket and then default to a
>> tpacket v1 in your switch statement. This doesn't seem right. Thus,
>> this should better be fixed in whatever way.
>
>
> It seems that "Opening a Tx-ring is NOT supported in TPACKET_V3" (see
> comment in packet_set_ring). So no one could be able to use the TX_OFF
> feature with a TPACKET_V3.
>
> The tpacket_snd function call some subroutines that call WARN and BUG (like
> in __packet_set_status),
> and other ones that simply ignore the tpacket v3 case (like in
> tpacket_fill_skb).
>
> What do you suggest ?
>
> I add a case that call WARN and BUG like in {__packet_set_status,
> __packet_get_status, packet_increment_rx_head} where we get tp_len in the
> tpacket_fill_skb function ?
>
> Or i let the code as is ?

Hmm, ok. Then maybe for the moment having it similar like the rest
seems fine. So, if one day someone adds a TX_RING to TPACKET_V3, this
needs to be addressed as well. Probably letting the code as is might
be okay for now.
--
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