netdev - Re: [PATCH net] datagram: When peeking datagrams with offset

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:   Thu, 17 Aug 2017 12:45:48 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Matthew Dawson <matthew@...systems.ca>
Cc:     Paolo Abeni <pabeni@...hat.com>,
        Network Development <netdev@...r.kernel.org>,
        "Macieira, Thiago" <thiago.macieira@...el.com>
Subject: Re: [PATCH net] datagram: When peeking datagrams with offset < 0
 don't skip empty skbs

>> @@ -169,14 +169,20 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock
>> *sk, int *peeked, int *off, int *err, struct sk_buff **last)
>>  {
>> +       bool peek_at_off = false;
>>         struct sk_buff *skb;
>> -       int _off = *off;
>> +       int _off = 0;
>> +
>> +       if (flags & MSG_PEEK && (*off) >= 0) {
>> +               peek_at_off = true;
>> +               _off = *off;
>> +       }
>>
>>         *last = queue->prev;
>>         skb_queue_walk(queue, skb) {
>>                 if (flags & MSG_PEEK) {
>> -                       if (_off >= skb->len && (skb->len || _off ||
>> -                                                skb->peeked)) {
>> +                       if (peek_at_off && _off >= skb->len &&
>> +                           (skb->len || _off || skb->peeked)) {
>                                 ^ I'm pretty sure we can remove this check
> (that skb->len is not zero) in this if statement.  If _off is zero, then skb-
>>len must also be zero (since _off >= skb->len, if _off is 0, skb->len <= 0.
> If skb->len can't be negative, then skb->len <= 0 => skb->len == 0).  If _off
> is not zero, then checking skb->len is redundant.

Good point.

>>                                 _off -= skb->len;
>>                                 continue;
>>                         }
> Is this queued to go in already? Or can I help by updating my patch with what
> was discussed here?  I can do that today if wanted.

It isn't. Please do if no one else has additional comments.