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: <20240121231615.13029448@elisabeth>
Date: Sun, 21 Jan 2024 23:16:15 +0100
From: Stefano Brivio <sbrivio@...hat.com>
To: Jon Maloy <jmaloy@...hat.com>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
 passt-dev@...st.top, lvivier@...hat.com, dgibson@...hat.com
Subject: Re: [RFC net-next] tcp: add support for read with offset when using
 MSG_PEEK

On Thu, 18 Jan 2024 17:22:52 -0500
Jon Maloy <jmaloy@...hat.com> wrote:

> On 2024-01-16 05:49, Paolo Abeni wrote:
> > On Thu, 2024-01-11 at 18:00 -0500, jmaloy@...hat.com wrote:  
> >> From: Jon Maloy <jmaloy@...hat.com>
> >>
> >> When reading received messages from a socket with MSG_PEEK, we may want
> >> to read the contents with an offset, like we can do with pread/preadv()
> >> when reading files. Currently, it is not possible to do that.  
> [...]
> >> +				err = -EINVAL;
> >> +				goto out;
> >> +			}
> >> +			peek_offset = msg->msg_iter.__iov[0].iov_len;
> >> +			msg->msg_iter.__iov = &msg->msg_iter.__iov[1];
> >> +			msg->msg_iter.nr_segs -= 1;
> >> +			msg->msg_iter.count -= peek_offset;
> >> +			len -= peek_offset;
> >> +			*seq += peek_offset;
> >> +		}  
> > IMHO this does not look like the correct interface to expose such
> > functionality. Doing the same with a different protocol should cause a
> > SIGSEG or the like, right?  
>
> I would expect doing the same thing with a different protocol to cause 
> an EFAULT, as it should. But I haven't tried it.

So, out of curiosity, I actually tried: the current behaviour is
recvmsg() failing with EFAULT, only as data is received (!), for TCP
and UDP with AF_INET, and for AF_UNIX (both datagram and stream).

EFAULT, however, is not in the list of "shall fail", nor "may fail"
conditions described by POSIX.1-2008, so there isn't really anything
that mandates it API-wise.

Likewise, POSIX doesn't require any signal to be delivered (and no
signals are delivered on Linux in any case: note that iov_base is not
dereferenced).

For TCP sockets only, passing a NULL buffer is already supported by
recv() with MSG_TRUNC (same here, Linux extension). This change would
finally make recvmsg() consistent with that TCP-specific bit.

> This is a change to TCP only, at least until somebody decides to 
> implement it elsewhere (why not?)

Side note, I can't really think of a reasonable use case for UDP -- it
doesn't quite fit with the notion of message boundaries.

Even letting alone the fact that passt(1) and pasta(1) don't need this
for UDP (no acknowledgement means no need to keep unacknowledged data
anywhere), if another application wants to do something conceptually
similar, we should probably target recvmmsg().

> > What about using/implementing SO_PEEK_OFF support instead?
>
> I looked at SO_PEEK_OFF, and it honestly looks both awkward and limited.

I think it's rather intended to skip headers with fixed size or
suchlike.

> We would have to make frequent calls to setsockopt(), something that 
> would beat much of the purpose of this feature.

...right, we would need to reset the SO_PEEK_OFF value at every
recvmsg(), which is probably even worse than the current overhead.

> I stand by my opinion here.
> This feature is simple, non-intrusive, totally backwards compatible and 
> implies no changes to the API or BPI.

My thoughts as well, plus the advantage for our user-mode networking
case is quite remarkable given how simple the change is.

> I would love to hear other opinions on this, though.
> 
> Regards
> /jon
> 
> >
> > Cheers,
> >
> > Paolo

-- 
Stefano


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ