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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Aug 2017 17:31:02 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Matthew Dawson <matthew@...systems.ca>
Cc:     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

On Mon, 2017-08-14 at 11:03 -0400, 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.

With the current code, the user space can disable peeking with offset
setting a negative offset value, after that peeking with offset has
been enabled. But only for UNIX sockets. I think the same should be
allowed for UDP sockets.

> > > I'm wondering adding an explicit SOCK_PEEK_OFF/MSG_PEEK_OFF socket flag
> > > would help simplyifing the code:
> 
> The behavior needs to be bifurcated between peeking with
> offset and without offset.
> 
> When peeking with offset is enabled by setting SO_PEEK_OFF,
> subsequent reads do move the offset, so the observed behavior
> is correct.
> 
> When sk->sk_peek_offset is negative, offset mode is disabled
> and the same packet must be read twice.
> 
> An explicit boolean flag to discern between the two may make
> the code simpler to understand, not sure whether that is logically
> required.

Yes, an explicit PEEK_OFF flag is just to keep the code simpler, so
that there is no need to add checks at every sk_peek_offset() call site
and the relevant logic can be fully encapsulated under the MSG_PEEK
branch in __skb_try_recv_from_queue(), I think/hope.
It's not a functional requirement.

Cheers,

Paolo