netdev - Re: [PATCH net] sctp: return a one-to-one type socket when doing peeloff

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]

Date:   Wed, 4 Mar 2020 17:38:24 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     "Leppanen, Jere (Nokia - FI/Espoo)" <jere.leppanen@...ia.com>
Cc:     network dev <netdev@...r.kernel.org>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        "michael.tuexen@...chi.franken.de" <michael.tuexen@...chi.franken.de>
Subject: Re: [PATCH net] sctp: return a one-to-one type socket when doing peeloff

On Wed, Mar 4, 2020 at 2:38 AM Leppanen, Jere (Nokia - FI/Espoo)
<jere.leppanen@...ia.com> wrote:
>
> On Mon, 2 Mar 2020, Xin Long wrote:
>
> > As it says in rfc6458#section-9.2:
> >
> >   The application uses the sctp_peeloff() call to branch off an
> >   association into a separate socket.  (Note that the semantics are
> >   somewhat changed from the traditional one-to-one style accept()
> >   call.)  Note also that the new socket is a one-to-one style socket.
> >   Thus, it will be confined to operations allowed for a one-to-one
> >   style socket.
> >
> > Prior to this patch, sctp_peeloff() returned a one-to-many type socket,
> > on which some operations are not allowed, like shutdown, as Jere
> > reported.
> >
> > This patch is to change it to return a one-to-one type socket instead.
>
> Thanks for looking into this. I like the patch, and it fixes my simple
> test case.
>
> But with this patch, peeled-off sockets are created by copying from a
> one-to-many socket to a one-to-one socket. Are you sure that that's
> not going to cause any problems? Is it possible that there was a
> reason why peeloff wasn't implemented this way in the first place?
I'm not sure, it's been there since very beginning, and I couldn't find
any changelog about it.

I guess it was trying to differentiate peeled-off socket from TCP style
sockets.

>
> With this patch there's no way to create UDP_HIGH_BANDWIDTH style
> sockets anymore, so the remaining references should probably be
> cleaned up:
>
> ./net/sctp/socket.c:1886:       if (!sctp_style(sk, UDP_HIGH_BANDWIDTH) && msg->msg_name) {
> ./net/sctp/socket.c:8522:       if (sctp_style(sk, UDP_HIGH_BANDWIDTH))
> ./include/net/sctp/structs.h:144:       SCTP_SOCKET_UDP_HIGH_BANDWIDTH,
>
> This patch disables those checks. The first one ignores a destination
> address given to sendmsg() with a peeled-off socket - I don't know
> why. The second one prevents listen() on a peeled-off socket.
My understanding is:
UDP_HIGH_BANDWIDTH is another kind of one-to-one socket, like TCP style.
it can get asoc by its socket when sending msg, doesn't need daddr.

Now I thinking to fix your issue in sctp_shutdown():

@@ -5163,7 +5163,7 @@ static void sctp_shutdown(struct sock *sk, int how)
        struct net *net = sock_net(sk);
        struct sctp_endpoint *ep;

-       if (!sctp_style(sk, TCP))
+       if (sctp_style(sk, UDP))
                return;

in this way, we actually think:
one-to-many socket: UDP style socket
one-to-one socket includes: UDP_HIGH_BANDWIDTH and TCP style sockets.