diff mbox series

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

Message ID 20210506184300.2241623-1-arjunroy.kdev@gmail.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net] tcp: Specify cmsgbuf is user pointer for receive zerocopy. | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers warning 3 maintainers not CCed: yoshfuji@linux-ipv6.org dsahern@kernel.org kuba@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 1 this patch: 1
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 7 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 1 this patch: 1
netdev/header_inline success Link

Commit Message

Arjun Roy May 6, 2021, 6:43 p.m. UTC 
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(+)

Comments

Soheil Hassas Yeganeh May 6, 2021, 6:50 p.m. UTC | #1 
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
>
Eric Dumazet May 6, 2021, 6:56 p.m. UTC | #2 
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
>
Arjun Roy May 6, 2021, 7 p.m. UTC | #3 
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
> >
diff mbox series

Patch

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) {