netdev - Re: [PATCH RFC v4 0/5] SCTP: Event skb list overhaul.

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 linux-cve-announce 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]

Message-ID: <20190314120432.GJ13343@localhost.localdomain>
Date:   Thu, 14 Mar 2019 09:04:32 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Xin Long <lucien.xin@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        network dev <netdev@...r.kernel.org>,
        Neil Horman <nhorman@...driver.com>
Subject: Re: [PATCH RFC v4 0/5] SCTP: Event skb list overhaul.

On Thu, Mar 14, 2019 at 03:51:59PM +0800, Xin Long wrote:
> On Thu, Mar 14, 2019 at 9:23 AM Marcelo Ricardo Leitner
> <marcelo.leitner@...il.com> wrote:
> >
> > On Wed, Mar 13, 2019 at 01:06:21PM -0300, Marcelo Ricardo Leitner wrote:
> > > On Mon, Mar 11, 2019 at 08:09:19PM -0700, David Miller wrote:
> > > >
> > > > This patch series eliminates the explicit reference to the skb list
> > > > implementation via skb->prev dereferences.
> > > >
> > > > The approach used is to pass a non-empty skb list around instead of an
> > > > event skb object which may or may not be on a list.
> > > >
> > > > I'd like to thank Marcelo Leitner, Xin Long, and Neil Horman for
> > > > reviewing previous versions of this series.
> > > >
> > > > Testing would be very much appreciated, in addition to the review of
> > > > course.
> > > >
> > > > v3 --> v4: Fix the logic in patch #4 so that we don't miss cases
> > > >                where we should add event to the on-stack temp list.
> > > >
> > > > Signed-off-by: David S. Miller <davem@...emloft.net>
> > >
> > > Code review LGTM too.
> > > Running some tests, should be done by the end of the day.
> >
> > Tests were good with the exception of one unrelated issue. ACK from my
> > side thus.
> >
> >
> > Xin, would you mind carrying forward this fix? I think we need to fix
> > the other sockopts too. Thanks!
> Sure, it seems to affect quite a few socket opts.

Thanks.

> Somehow I made sense of it before.

For me too :-)

> So with this patch, asoc_id will be completed ignored by TCP type socket?

Yes. It already is when looking for the asoc, by sctp_id2assoc(). Just
need to re-ignore it if there is no asoc yet.

> 
> >
> > ---8<---
> >
> > From ddba48476b343dce84a82036e7914a1f7ac3f0c8 Mon Sep 17 00:00:00 2001
> > Message-Id: <ddba48476b343dce84a82036e7914a1f7ac3f0c8.1552526507.git.marcelo.leitner@...il.com>
> > From: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> > Date: Wed, 13 Mar 2019 22:13:52 -0300
> > Subject: [PATCH] net/sctp: fix ignoring asoc_id for tcp-style sockets on
> >  setsockopt
> >
> > Currently if the user pass an invalid asoc_id to SCTP_DEFAULT_SEND_PARAM
> > on a TCP-style socket, it will silently ignore the new parameters.
> > That's because after not finding an asoc, it is checking asoc_id against
> > the known values of CURRENT/FUTURE/ALL values and that fails to match.
> >
> > IOW, if the user supplies an invalid asoc id or not, it should either
> > match the current asoc or the socket itself so that it will inherit
> > these later. Fixes it by forcing asoc_id to SCTP_FUTURE_ASSOC in case it
> > is a TCP-style socket without an asoc, so that the values get set on the
> > socket.
> >
> > Fixes: 707e45b3dc5a ("sctp: use SCTP_FUTURE_ASSOC and add SCTP_CURRENT_ASSOC for SCTP_DEFAULT_SEND_PARAM sockopt")
> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> > ---
> >  net/sctp/socket.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 533207dbeae955604a6304a8f67b65c9f580fd05..85205a2d9a7dc5c289a94bf4672b240040cbb546 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -3024,6 +3024,9 @@ static int sctp_setsockopt_default_send_param(struct sock *sk,
> >                 return 0;
> >         }
> >
> > +       if (sctp_style(sk, TCP))
> > +               info.sinfo_assoc_id = SCTP_FUTURE_ASSOC;
> > +
> >         if (info.sinfo_assoc_id == SCTP_FUTURE_ASSOC ||
> >             info.sinfo_assoc_id == SCTP_ALL_ASSOC) {
> >                 sp->default_stream = info.sinfo_stream;
> > --
> > 2.20.1
> >