| Message ID | 56d3373c1c5007d776fcd5de4523f4b9da341fb6.1552679409.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: untag user pointers passed to the kernel | expand |
On 03/15/2019 12:51 PM, 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..89db3b4fc753 100644 > --- a/net/ipv4/tcp.c > +++ b/net/ipv4/tcp.c > @@ -1758,6 +1758,8 @@ static int tcp_zerocopy_receive(struct sock *sk, > int inq; > int ret; > > + address = untagged_addr(address); > + > if (address & (PAGE_SIZE - 1) || address != zc->address) The second test will fail, if the top bits are changed in address but not in zc->address > return -EINVAL; > >
On Fri, Mar 15, 2019 at 9:03 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > On 03/15/2019 12:51 PM, 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..89db3b4fc753 100644 > > --- a/net/ipv4/tcp.c > > +++ b/net/ipv4/tcp.c > > @@ -1758,6 +1758,8 @@ static int tcp_zerocopy_receive(struct sock *sk, > > int inq; > > int ret; > > > > + address = untagged_addr(address); > > + > > if (address & (PAGE_SIZE - 1) || address != zc->address) > > The second test will fail, if the top bits are changed in address but not in zc->address Will fix in v12, thanks Eric! > > > return -EINVAL; > > > > >
On Mon, Mar 18, 2019 at 2:14 PM Andrey Konovalov <andreyknvl@google.com> wrote: > > On Fri, Mar 15, 2019 at 9:03 PM Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > > > > > > On 03/15/2019 12:51 PM, 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..89db3b4fc753 100644 > > > --- a/net/ipv4/tcp.c > > > +++ b/net/ipv4/tcp.c > > > @@ -1758,6 +1758,8 @@ static int tcp_zerocopy_receive(struct sock *sk, > > > int inq; > > > int ret; > > > > > > + address = untagged_addr(address); > > > + > > > if (address & (PAGE_SIZE - 1) || address != zc->address) > > > > The second test will fail, if the top bits are changed in address but not in zc->address > > Will fix in v12, thanks Eric! Looking at the code, what's the point of this address != zc->address check? Should I just remove it? > > > > > > return -EINVAL; > > > > > > > >
On Mon, Mar 18, 2019 at 6:17 AM Andrey Konovalov <andreyknvl@google.com> wrote: > > Looking at the code, what's the point of this address != zc->address > check? Should I just remove it? No you must not remove it. The test detects if a u64 ->unsigned long conversion might have truncated bits. Quite surprisingly some people still use 32bit kernels. The ABI is 64bit only, because we did not want to have yet another compat layer. struct tcp_zerocopy_receive { __u64 address; /* in: address of mapping */ __u32 length; /* in/out: number of bytes to map/mapped */ __u32 recv_skip_hint; /* out: amount of bytes to skip */ };
On Mon, Mar 18, 2019 at 3:45 PM Eric Dumazet <edumazet@google.com> wrote: > > On Mon, Mar 18, 2019 at 6:17 AM Andrey Konovalov <andreyknvl@google.com> wrote: > > > > > Looking at the code, what's the point of this address != zc->address > > check? Should I just remove it? > > No you must not remove it. > > The test detects if a u64 ->unsigned long conversion might have truncated bits. > > Quite surprisingly some people still use 32bit kernels. > > The ABI is 64bit only, because we did not want to have yet another compat layer. > > struct tcp_zerocopy_receive { > __u64 address; /* in: address of mapping */ > __u32 length; /* in/out: number of bytes to map/mapped */ > __u32 recv_skip_hint; /* out: amount of bytes to skip */ > }; Ah, got it, thanks! I'll add a comment here then, otherwise this looks confusing.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 6baa6dc1b13b..89db3b4fc753 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1758,6 +1758,8 @@ static int tcp_zerocopy_receive(struct sock *sk, int inq; int ret; + address = untagged_addr(address); + if (address & (PAGE_SIZE - 1) || address != zc->address) return -EINVAL;
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(+)