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

Message-ID: <CAF=yD-+k79=fykYs5x5-7AvE85+JPXv=NcfDy8X-4AGP8P85Pw@mail.gmail.com>
Date:   Mon, 14 Aug 2017 12:33:48 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Thiago Macieira <thiago.macieira@...el.com>
Cc:     Matthew Dawson <matthew@...systems.ca>,
        Paolo Abeni <pabeni@...hat.com>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH net] datagram: When peeking datagrams with offset < 0
 don't skip empty skbs

On Mon, Aug 14, 2017 at 12:06 PM, Thiago Macieira
<thiago.macieira@...el.com> wrote:
> On Monday, 14 August 2017 08:03:50 PDT Willem de Bruijn wrote:
>> > I'm actually surprised that only unix sockets can have negative values.
>> > Is
>> > there a reason for that?  I had assumed that sk_set_peek_off would allow
>> > negative values as the code already has to support negative values due to
>> > what the initial value is.
>>
>> A negative initial value indicates that PEEK_OFF is disabled. It only
>> makes sense to peek from a positive offset from the start of the data.
>
> But here's a question: if the peek offset is equal to the length, should the
> reading return an empty datagram? This would indicate to the caller that there
> was a datagram there, which was skipped over.

In the general case, no, it should read at the offset, which is the next skb.

Zero length packets are a special case. This did come up before and
we chose to signal their existence in the queue by returning 0 for each
once, even in the offset-enabled mode.

Since we only need to change no-offset semantics to fix this bug,
I would not change this behavior, which is also expected by some
applications by now.

>
> That's how we deal with empty datagrams anyway.

What is? With no-offset and a zero payload skb at the head, peek
or recv returns 0, right?