On Mon, Mar 25, 2019 at 2:54 PM Kevin Brodsky <kevin.brodsky@xxxxxxx> 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@xxxxxxxxxx> > >> --- > >> 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
