netdev - Re: Stack sends oversize UDP packet to the driver

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:   Tue, 22 Jan 2019 10:28:58 -0800
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Michael Chan <michael.chan@...adcom.com>,
        Daniel Axtens <dja@...ens.net>
Cc:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: Stack sends oversize UDP packet to the driver

On Mon, Jan 21, 2019 at 4:59 PM Michael Chan <michael.chan@...adcom.com> wrote:
>
> On Mon, Jan 21, 2019 at 4:36 PM Daniel Axtens <dja@...ens.net> wrote:
>
> > I hit a similar sounding issue on a bnx2x - see commit
> > 8914a595110a6eca69a5e275b323f5d09e18f4f9
> >
> > In that case, a GSO packet with gso_size too large for the firmware was
> > coming to the bnx2x driver from an ibmveth device via Open vSwitch. I
> > also toyed with a fix in the stack and ended up fixing just the driver.
>
> Hi Daniel, yes I remember the issue that you reported at that time.
> The current issue is similar but for non-TSO UDP packets.
>
> >
> > I was hoping to get a generic fix in to the stack afterwards, but didn't
> > get anything finished. Looking back at old branches, it looks like I
> > considered adding MTU validation to validate_xmit_skb,
>
> MTU validation in validate_xmit_skb() would definitely prevent the
> current reported issue.  Perhaps we can add a WARN() when an invalid
> length is detected so that the underlying issue will be reported and
> fixed.  The current issue is that somehow the dst of the SKB gets
> changed to "lo" and fragmentation is not done for the proper output
> device.  Thanks.

[Looks like my earlier reply didn't make it to most of the people in
the list, So let me include that here also]

>>>
We have been battling with something similar. The suspicion is that
when an egress packet gets transferred from L4 to L3 some strange
sequence of event causes dst invalidation which results into dst
device to change to 'lo' which has the MTU of 65k, this makes the
stack pass to a frame larger than the MTU to the driver which results
into the undesired outcome you have mentioned.

I have cooked few patches to address this issue, however, I'm finding
it really hard to reproduce this issue in my test environment. If you
could try them and see if those fixes the issue, we can refine them if
necessary and eventually formalize patches.
<<<

Actually while looking at this issue I did look at the fix Daniel
mentioned (8914a595110a6eca69a5e275b323f5d09e18f4f9) and wanted to
reach out to him to see if the solution that I'm trying to implement
in the stack works in that scenario too.

The idea behind the fix is very simple and it is to create a dst-only
(unregistered) device with a very low MTU and use it instead of 'lo'
while invalidating the dst. This would make it *not* forward packets
to driver which might need fragmentation.

Attaching the patches to this mail. [Let me know if I should provide
them in a different way]

thanks,
--mahesh..

Download attachment "0001-loopback-create-blackhole-net-device-similar-to-loop.patch" of type "application/octet-stream" (4587 bytes)

Download attachment "0002-blackhole_netdev-use-blackhole_netdev-to-invalidate-.patch" of type "application/octet-stream" (2003 bytes)