netdev - Missing skb->dst with flow offloading

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

Date:   Wed, 30 May 2018 02:01:05 +0200
From:   "Jason A. Donenfeld" <Jason@...c4.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     Felix Fietkau <nbd@....name>, netfilter-devel@...r.kernel.org,
        Netdev <netdev@...r.kernel.org>, openwrt-devel@...ts.openwrt.org,
        Jaap Buurman <jaapbuurman@...il.com>,
        WireGuard mailing list <wireguard@...ts.zx2c4.com>
Subject: Missing skb->dst with flow offloading

Hey Pablo,

Some OpenWRT people have reported to me that there's a crash when
enabling flow offloading, because I rely on skb_dst(skb) being
non-null in ndo_start_xmit. The fix in my code for this is very
simple:

- mtu = dst_mtu(skb_dst(skb));
+ dst = skb_dst(skb);
+ mtu = dst ? dst_mtu(dst) : dev->mtu;

I can make this change, but I wanted to be certain first that omitting
the dst in the skb is intentional on your part. (If so, there might be
other drivers to fix as well.) In tracing this, it looks like a packet
that's forwarded from a flow offloaded interface to a virtual
interface gets diverted immediately via neigh_xmit, where it is then
passed to a virtual interface via dev_queue_xmit. I can't see anywhere
along this path a call to skb_dst_set. Perhaps this is intended, as
flow offloading is supposed to skip the routing table? Or is there an
oversight in the new flow offloading code?

I'd appreciate your input, so that I can make the appropriate change
-- or not -- to my code.

Regards,
Jason