[PATCH v13 09/20] net, arm64: untag user pointers in tcp_zerocopy_receive

[Andrey Konovalov]

On Mon, Mar 25, 2019 at 2:54 PM Kevin Brodsky <kevin.brodsky at arm.com> wrote:
>
> On 22/03/2019 12:04, Catalin Marinas wrote:
> > On Wed, Mar 20, 2019 at 03:51:23PM +0100, Andrey Konovalov wrote:
> >> This patch is a part of a series that extends arm64 kernel ABI to allow to
> >> pass tagged user pointers (with the top byte set to something else other
> >> than 0x00) as syscall arguments.
> >>
> >> tcp_zerocopy_receive() uses provided user pointers for vma lookups, which
> >> can only by done with untagged pointers.
> >>
> >> Untag user pointers in this function.
> >>
> >> Signed-off-by: Andrey Konovalov <andreyknvl at google.com>
> >> ---
> >>   net/ipv4/tcp.c | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> >> index 6baa6dc1b13b..855a1f68c1ea 100644
> >> --- a/net/ipv4/tcp.c
> >> +++ b/net/ipv4/tcp.c
> >> @@ -1761,6 +1761,8 @@ static int tcp_zerocopy_receive(struct sock *sk,
> >>      if (address & (PAGE_SIZE - 1) || address != zc->address)
> >>              return -EINVAL;
> >>
> >> +    address = untagged_addr(address);
> >> +
> >>      if (sk->sk_state == TCP_LISTEN)
> >>              return -ENOTCONN;
> > I don't think we need this patch if we stick to Vincenzo's ABI
> > restrictions. Can zc->address be an anonymous mmap()? My understanding
> > of TCP_ZEROCOPY_RECEIVE is that this is an mmap() on a socket, so user
> > should not tag such pointer.
>
> Good point, I hadn't looked into the interface properly. The `vma->vm_ops !=
> &tcp_vm_ops` check just below makes sure that the mapping is specifically tied to a
> TCP socket, so definitely not included in the ABI relaxation.
>
> > We want to allow tagged pointers to work transparently only for heap and
> > stack, hence the restriction to anonymous mmap() and those addresses
> > below sbrk(0).

Right, I'll drop this patch, thanks for noticing!

>
> That's not quite true: in the ABI relaxation v2, all private mappings that are either
> anonymous or backed by a regular file are included. The scope is quite a bit larger
> than heap and stack, even though this is what we're primarily interested in for now.
>
> Kevin