| Message ID | 2280b62096ce1fa5c9e9429d18f08f82f4be1b0b.1553093421.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: untag user pointers passed to the kernel | expand |
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. 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).
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
On Mon, Mar 25, 2019 at 2:54 PM Kevin Brodsky <kevin.brodsky@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@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
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;
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(+)