netdev - Re: [Patch net] net_sched: refetch skb protocol for each filter

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:   Sun, 13 Jan 2019 13:08:40 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Jamal Hadi Salim <jhs@...atatu.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Martin Olsson <martin.olsson+netdev@...torsecurity.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Lucas Bates <lucasb@...atatu.com>
Subject: Re: [Patch net] net_sched: refetch skb protocol for each filter

On Sat, Jan 12, 2019 at 7:41 AM Jamal Hadi Salim <jhs@...atatu.com> wrote:
>
> On 2019-01-12 7:23 a.m., Jamal Hadi Salim wrote:
>
> > Do we have a test case for a setup like this in tdc?
> > i.e incoming tagged and then vlan popped by action?
> > Otherwise a test with IFE which resets the ethertype
> > would be sufficient i.e just something that will messup
> > with skb->protocol.
>
> And here is a slightly complex test with IFE.
> Wanted to show both reclassify and continue in play
> as well as something that change skb->protocol.
>

I don't know why you need a complex one, Martin's
test case is pretty simple (as I already sent to you).

Also, you can add two printk()'s around the skb_vlan_pop()
in tcf_vlan_act() to see the difference of tc_skb_protocol()
return values before and after. I tried, it clearly shows
ETH_P_8021Q and ETH_P_IP.

Of course, it could be tc_skb_protocol() which is wrong,
as skb->protocol stays same.

This patch is always correct despite of tc_skb_protocol():

1. If tc_skb_protocol() is wrong, this patch fixes nothing,
and harms nothing.

2. If tc_skb_protocol() is correct, this patch fixes a bug.

Changing tc_skb_protocol() is much more risky than this
patch. You fixed a very similar bug before:

commit 619fe32640b4b01f370574d50344ae0f62689816
Author: Jamal Hadi Salim <jhs@...atatu.com>
Date:   Thu Feb 18 07:38:04 2016 -0500

    net_sched fix: reclassification needs to consider ether protocol changes

which also implies tc_skb_protocol() is correct.

Thanks.