lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Tue, 12 May 2020 17:09:32 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Christoph Paasch <cpaasch@...le.com>
Subject: Re: [RFC PATCH 1/3] mptcp: add new sock flag to deal with join
 subflows

Hi,

On Tue, 2020-05-12 at 07:46 -0700, Eric Dumazet wrote:
> On Tue, May 12, 2020 at 7:11 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > MP_JOIN subflows must not land into the accept queue.
> > Currently tcp_check_req() calls an mptcp specific helper
> > to detect such scenario.
> > 
> > Such helper leverages the subflow context to check for
> > MP_JOIN subflows. We need to deal also with MP JOIN
> > failures, even when the subflow context is not available
> > due to allocation failure.
> > 
> > A possible solution would be changing the syn_recv_sock()
> > signature to allow returning a more descriptive action/
> > error code and deal with that in tcp_check_req().
> > 
> > Since the above need is MPTCP specific, this patch instead
> > uses a TCP socket hole to add an MPTCP specific flag.
> > Such flag is used by the MPTCP syn_recv_sock() to tell
> > tcp_check_req() how to deal with the request socket.
> > 
> > This change is a no-op for !MPTCP build, and makes the
> > MPTCP code simpler. It allows also the next patch to deal
> > correctly with MP JOIN failure.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> > ---
> >  include/linux/tcp.h      |  1 +
> >  include/net/mptcp.h      | 17 ++++++++++-------
> >  net/ipv4/tcp_minisocks.c |  2 +-
> >  net/mptcp/protocol.c     |  7 -------
> >  net/mptcp/subflow.c      |  2 ++
> >  5 files changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> > index e60db06ec28d..dc12c59db41e 100644
> > --- a/include/linux/tcp.h
> > +++ b/include/linux/tcp.h
> > @@ -385,6 +385,7 @@ struct tcp_sock {
> >                            */
> >  #if IS_ENABLED(CONFIG_MPTCP)
> >         bool    is_mptcp;
> > +       bool    drop_req;
> >  #endif
> 
> This looks like this should only be needed in struct tcp_request_sock ?
> 
> Does this information need to be kept in the TCP socket after accept() ?

Thank you for the feedback, indeed you are right! this should be moved
inside tcp_request_sock. We still have 2 bytes hole there. I will keep
the flag under a CONFIG_MPTCP conditional.

Would you be ok with such approach, or do you think we should look for
some other schema (like the alternative mentioned in the cover letter)?

Cheers,

Paolo


Powered by blists - more mailing lists