netdev - Re: tc flower IP address keys

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:   Fri, 11 Jan 2019 13:20:01 +0000
From:   Edward Cree <ecree@...arflare.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
CC:     Jamal Hadi Salim <jhs@...atatu.com>, Jiri Pirko <jiri@...nulli.us>,
        netdev <netdev@...r.kernel.org>
Subject: Re: tc flower IP address keys

On 11/01/19 00:31, Cong Wang wrote:
> I don't follow this question. The code you quoted from
> fl_init_dissector() is correct, as it merely stores the offsets of ipv4/ipv6
> fields. If somewhere we use it without checking control.addr_type,
> then it is a bug there.
My point is that it shouldn't store the offset of the ipv6 field if the
 ipv6 field isn't actually used.  I've been looking at driver implementations
 of tc flower offload, and it seems like the usual pattern is to use
 dissector_uses_key() to determine whether a field is being filtered on; then
 the drivers are having to break that pattern in the case of IP addrs and
 check this other bit of info in control.addr_type.
When initially creating the flow dissector from the tc filter netlink message,
 we have the information that the IPv4 address was specified and the IPv6
 wasn't.  But rather than putting that information in used_keys (like we do
 with _all_ the other information about "was this field specified"), we stuff
 it in this special other field.  Which is still maskable despite that not
 making sense, so everyone has to check its mask is all-ones too.
So I suppose it's not a bug (I was wrong to call it that), but it is a wart.
Why is it so hard for a flower filter to just not set an offset for IPV6_ADDRS
 when it's an IPv4 address filter?  I know that's not how it works right now,
 but _why_ not?

> I don't see anything particular here, we use IPv4/IPv6 address union
> in so many other places in networking, I don't see why this one is so
> special to you. :)
Usually we don't also have a mask that's similarly unioned and is part of
 identifying whether a field is used.
Either the IP addresses are a single unified datum, in which case there should
 just be a single FLOW_DISSECTOR_KEY_IP_ADDRS, or they're separate, in which
 case the distinction between FLOW_DISSECTOR_KEY_IPV4_ADDRS and ..._IPV6_ADDRS
 in used_keys should be meaningful and informative.

-Ed