netdev - Re: Why do we prefer skb->priority to tc filters?

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]

Date:	Thu, 12 Mar 2015 09:59:02 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	David Miller <davem@...emloft.net>
Subject: Re: Why do we prefer skb->priority to tc filters?

On Wed, Mar 11, 2015 at 5:00 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Wed, 2015-03-11 at 15:12 -0700, Cong Wang wrote:
>
>> I knew we can modify skb->priority in a few ways, for example skbedit.
>>
>> That is not my concern, all what I am thinking is there is some
>> way in application layer to bypass our tc filters, which is not expected
>> to happen for me. Given our specific case, I want to propose to clear
>> skb->priority after moving out of a netns:
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 962ee9d..2301f01 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1694,6 +1694,7 @@ int __dev_forward_skb(struct net_device *dev,
>> struct sk_buff *skb)
>>         }
>>
>>         skb_scrub_packet(skb, true);
>> +       skb->priority = 0;
>>         skb->protocol = eth_type_trans(skb, dev);
>>         skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
>
> This looks more reasonable than your prior change to fq_codel, sfq, and
> others.


My previous change is only for discussion, never mean to send out.

>
> If you use mavclan or ipvlan, not all paths will use __dev_forward_skb()
>

That is intentional, because I am not sure about stacked device
like macvlan or tunnel, it might make some sense to keep that value.

> So I am guessing you use veth maybe. Who knows.
>


Exactly, otherwise I would probably just change skb_scrub_packet(). ;)


> Note that you could instead use :
>
>  skb->priority = skb->priority & TC_PRIO_MAX;
>
> This way, TC_PRIO_CONTROL legitimate traffic would not be downgraded to
> TC_PRIO_BESTEFFORT.
>
> This all looks like a policy decision, and we probably should not hard
> code it in the kernel : Some users have trusted applications running in
> containers.
>

For veth case, I think we don't need to keep TC_PRIO_* either, because
the packet will normally go to Rx path after that, where these values make
no sense.

(For my own case, we redirect it to eth0, but still it goes out of its
original netns).

Thanks.
--
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