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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_f-OB+_vPxefcCKAM=hMVNYRDp+wYv+xYz9agRmaEmNLw@mail.gmail.com>
Date:   Thu, 14 Mar 2019 15:51:59 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...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 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.
Somehow I made sense of it before.
So with this patch, asoc_id will be completed ignored by TCP type socket?

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