[net] tcp: Specify cmsgbuf is user pointer for receive zerocopy.

[net] tcp: Specify cmsgbuf is user pointer for receive zerocopy.

[Arjun Roy]

From: Arjun Roy <arjunroy@google.com>

A prior change introduces separate handling for ->msg_control
depending on whether the pointer is a kernel or user pointer. However,
it does not update tcp receive zerocopy (which uses a user pointer),
which can cause faults when the improper mechanism is used.

This patch simply annotates tcp receive zerocopy's use as explicitly
being a user pointer.

Fixes: 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control")
Signed-off-by: Arjun Roy <arjunroy@google.com>
---
 net/ipv4/tcp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e14fd0c50c10..f1c1f9e3de72 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2039,6 +2039,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk,
 		(__kernel_size_t)zc->msg_controllen;
 	cmsg_dummy.msg_flags = in_compat_syscall()
 		? MSG_CMSG_COMPAT : 0;
+	cmsg_dummy.msg_control_is_user = true;
 	zc->msg_flags = 0;
 	if (zc->msg_control == msg_control_addr &&
 	    zc->msg_controllen == cmsg_dummy.msg_controllen) {
-- 
2.31.1.607.g51e8a6a459-goog

Re: [net] tcp: Specify cmsgbuf is user pointer for receive zerocopy.

[Soheil Hassas Yeganeh]

On Thu, May 6, 2021 at 2:43 PM Arjun Roy <arjunroy.kdev@gmail.com> wrote:
>
> From: Arjun Roy <arjunroy@google.com>
>
> A prior change introduces separate handling for ->msg_control
> depending on whether the pointer is a kernel or user pointer. However,
> it does not update tcp receive zerocopy (which uses a user pointer),
> which can cause faults when the improper mechanism is used.
>
> This patch simply annotates tcp receive zerocopy's use as explicitly
> being a user pointer.
>
> Fixes: 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control")
> Signed-off-by: Arjun Roy <arjunroy@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Thank you for the fix!

> ---
>  net/ipv4/tcp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e14fd0c50c10..f1c1f9e3de72 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2039,6 +2039,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk,
>                 (__kernel_size_t)zc->msg_controllen;
>         cmsg_dummy.msg_flags = in_compat_syscall()
>                 ? MSG_CMSG_COMPAT : 0;
> +       cmsg_dummy.msg_control_is_user = true;
>         zc->msg_flags = 0;
>         if (zc->msg_control == msg_control_addr &&
>             zc->msg_controllen == cmsg_dummy.msg_controllen) {
> --
> 2.31.1.607.g51e8a6a459-goog
>

Re: [net] tcp: Specify cmsgbuf is user pointer for receive zerocopy.

[Eric Dumazet]

On Thu, May 6, 2021 at 8:43 PM Arjun Roy <arjunroy.kdev@gmail.com> wrote:
>
> From: Arjun Roy <arjunroy@google.com>
>
> A prior change introduces separate handling for ->msg_control
> depending on whether the pointer is a kernel or user pointer. However,
> it does not update tcp receive zerocopy (which uses a user pointer),
> which can cause faults when the improper mechanism is used.
>
> This patch simply annotates tcp receive zerocopy's use as explicitly
> being a user pointer.
>
> Fixes: 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control")

This Fixes: tag is wrong.

When this commit was merged, tcp_zc_finalize_rx_tstamp() was not yet there.

> Signed-off-by: Arjun Roy <arjunroy@google.com>
> ---
>  net/ipv4/tcp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e14fd0c50c10..f1c1f9e3de72 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2039,6 +2039,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk,
>                 (__kernel_size_t)zc->msg_controllen;
>         cmsg_dummy.msg_flags = in_compat_syscall()
>                 ? MSG_CMSG_COMPAT : 0;
> +       cmsg_dummy.msg_control_is_user = true;
>         zc->msg_flags = 0;
>         if (zc->msg_control == msg_control_addr &&
>             zc->msg_controllen == cmsg_dummy.msg_controllen) {
> --
> 2.31.1.607.g51e8a6a459-goog
>

Re: [net] tcp: Specify cmsgbuf is user pointer for receive zerocopy.

[Arjun Roy]

On Thu, May 6, 2021 at 11:56 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Thu, May 6, 2021 at 8:43 PM Arjun Roy <arjunroy.kdev@gmail.com> wrote:
> >
> > From: Arjun Roy <arjunroy@google.com>
> >
> > A prior change introduces separate handling for ->msg_control
> > depending on whether the pointer is a kernel or user pointer. However,
> > it does not update tcp receive zerocopy (which uses a user pointer),
> > which can cause faults when the improper mechanism is used.
> >
> > This patch simply annotates tcp receive zerocopy's use as explicitly
> > being a user pointer.
> >
> > Fixes: 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control")
>
> This Fixes: tag is wrong.
>
> When this commit was merged, tcp_zc_finalize_rx_tstamp() was not yet there.
>

Makes sense - I'll send out an amended v2 patch that tags
'7eeba1706eba' instead.

-Arjun

> > Signed-off-by: Arjun Roy <arjunroy@google.com>
> > ---
> >  net/ipv4/tcp.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index e14fd0c50c10..f1c1f9e3de72 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -2039,6 +2039,7 @@ static void tcp_zc_finalize_rx_tstamp(struct sock *sk,
> >                 (__kernel_size_t)zc->msg_controllen;
> >         cmsg_dummy.msg_flags = in_compat_syscall()
> >                 ? MSG_CMSG_COMPAT : 0;
> > +       cmsg_dummy.msg_control_is_user = true;
> >         zc->msg_flags = 0;
> >         if (zc->msg_control == msg_control_addr &&
> >             zc->msg_controllen == cmsg_dummy.msg_controllen) {
> > --
> > 2.31.1.607.g51e8a6a459-goog
> >