netdev - RE: SCTP doesn't seem to let you 'cancel' a blocking accept()

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: Fri, 7 Jun 2024 08:36:23 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Xin Long' <lucien.xin@...il.com>
CC: linux-sctp <linux-sctp@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
Subject: RE: SCTP doesn't seem to let you 'cancel' a blocking accept()

From: Xin Long
> Sent: 06 June 2024 21:15
> 
> On Mon, Jun 3, 2024 at 11:42 AM David Laight <David.Laight@...lab.com> wrote:
> >
> > In a multithreaded program it is reasonable to have a thread blocked in accept().
> > With TCP a subsequent shutdown(listen_fd, SHUT_RDWR) causes the accept to fail.
> > But nothing happens for SCTP.
> >
> > I think the 'magic' happens when tcp_disconnect() calls inet_csk_listen_stop(sk)
> > but sctp_disconnect() is an empty function and nothing happens.
> >
> > I can't see any calls to inet_csk_listen_stop() in the sctp code - so I suspect
> > it isn't possible at all.
...
> >
> > I also suspect that a blocking connect() can't be cancelled either?
>
> For connecting socket, it calls sctp_shutdown() where SHUT_WR causes
> the asoc to enter SHUTDOWN_SENT and cancel the blocking connect().

I'll test that later - the test I was running always connects.
I'm porting some kernel code that used signals to unblock synchronous
calls to userspace where you can't signal a thread.
The only problem with the kernel version is secure boot and driver
signing (especially for the windows build!).

> > Clearly the application can avoid the issue by using poll() and an
> > extra eventfd() for the wakeup - but it is all a faff for code that
> > otherwise straight forward.
>
> I will try to prepare a patch to solve this for sctp accept() like:

I'll test it for you.

> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index c67679a41044..f270a0a4c65d 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4834,10 +4834,13 @@ int sctp_inet_connect(struct socket *sock,
> struct sockaddr *uaddr,
>         return sctp_connect(sock->sk, uaddr, addr_len, flags);
>  }
> 
> -/* FIXME: Write comments. */
>  static int sctp_disconnect(struct sock *sk, int flags)
>  {
> -       return -EOPNOTSUPP; /* STUB */
> +       if (!sctp_style(sk, TCP))
> +               return -EOPNOTSUPP;
> +
> +       sk->sk_shutdown |= RCV_SHUTDOWN;
> +       return 0;

I think you need to call something to unblock the thread as well
as changing the state.

...
> -       if (!sctp_sstate(sk, LISTENING)) {

Any chance of making it much clearer that this is testing
		if (sk->sk_state == TCP_LISTEN)

The token-pasting though
	SCTP_SS_CLOSED         = TCP_CLOSE,
	SCTP_SS_LISTENING      = TCP_LISTEN,
	SCTP_SS_ESTABLISHING   = TCP_SYN_SENT,
	SCTP_SS_ESTABLISHED    = TCP_ESTABLISHED,
	SCTP_SS_CLOSING        = TCP_CLOSE_WAIT,
makes grepping for changes to sk_state pretty impossible.

You might argue that the sk_state values should be protocol neutral,
and that the wrapper gives strong typing - but together they make
the code hard to scan.

The strong typing could be maintained by changing the constants to
	SCTP_SS_TCP_CLOSE = TCP_CLOSE
(etc) so that grepping for the constants still works.

I keep thinking of ways to do strongly typed enum in C.
The main options seem to be embedding the value in a struct
or using a pointer to a struct.
Neither is ideal.

OTOH the compiler can't default to strongly typed enum.
Although perhaps that could be a per-enum attribute.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)