netdev - Re: [PATCH net-next 3/3] udp: only use paged allocation with scatter-gather

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]

Message-ID: <CAF=yD-KUgKoaV-CXcZpRLBVPykkpGmBAm59C=+OWFq4wd6g5xg@mail.gmail.com>
Date:   Tue, 15 May 2018 10:14:20 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Network Development <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next 3/3] udp: only use paged allocation with scatter-gather

On Mon, May 14, 2018 at 7:45 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
>
>
> On 05/14/2018 04:30 PM, Willem de Bruijn wrote:
>
>> I don't quite follow. The reported crash happens in the protocol layer,
>> because of this check. With pagedlen we have not allocated
>> sufficient space for the skb_put.
>>
>>                 if (!(rt->dst.dev->features&NETIF_F_SG)) {
>>                         unsigned int off;
>>
>>                         off = skb->len;
>>                         if (getfrag(from, skb_put(skb, copy),
>>                                         offset, copy, off, skb) < 0) {
>>                                 __skb_trim(skb, off);
>>                                 err = -EFAULT;
>>                                 goto error;
>>                         }
>>                 } else {
>>                         int i = skb_shinfo(skb)->nr_frags;
>>
>> Are you referring to a separate potential issue in the gso layer?
>> If a bonding device advertises SG, but a slave does not, then
>> skb_segment on the slave should build linear segs? I have not
>> tested that.
>
> Given that the device attribute could change under us, we need to not
> crash, even if initially we thought NETIF_F_SG was available.
>
> Unless you want to hold RTNL in UDP xmit :)
>
> Ideally, GSO should be always on, as we did for TCP.
>
> Otherwise, I can guarantee syzkaller will hit again.

Ah, right. Thanks, Eric!

I'll read that feature bit only once.