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

Kevin Brodsky
2019-03-25T13:54:46Z

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@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).

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