From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Allown listen(2) only on sockets of type SOCK_STREAM or SOCK_SEQPACKET
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Sun, 31 Mar 2024 16:56:21 +0300

On Sun, Mar 31, 2024 at 03:47:09PM +0200, Alexander Bluhm wrote:
> On Sun, Mar 31, 2024 at 04:05:57PM +0300, Vitaliy Makkoveev wrote:
> > Syzkaller found that SOCK_DGRAM coould became listening socket, which is
> > wrong.
> > 
> > 1. https://syzkaller.appspot.com/bug?extid=00450333592fcd38c6fe
> 
> I looked in 4.4BSD, it looks like this was always allowed.  The
> only thing it did, was to set SO_ACCEPTCONN.  And the code did not
> care.  But now, with locking and assert it gets relevant.
> 
> Setting SO_ACCEPTCONN on SOCK_DGRAM makes not sense.  Better fail
> early.
> 

Guess this check was initially missing. listen(2) man page permits
listen(2) calls only on sockets of type SOCK_STREAM and SOCK_SEQPACKET:

DESCRIPTION
	...
	The listen() call applies only to sockets of type SOCK_STREAM or
	SOCK_SEQPACKET.

ERRORS
	listen() will fail if:
	...

	[EOPNOTSUPP]       The socket is not of a type that supports the
	                   operation listen().

> OK bluhm@
> 
> > Index: sys/kern/uipc_socket.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.323
> > diff -u -p -r1.323 uipc_socket.c
> > --- sys/kern/uipc_socket.c	27 Mar 2024 22:47:53 -0000	1.323
> > +++ sys/kern/uipc_socket.c	31 Mar 2024 13:04:01 -0000
> > @@ -231,6 +231,14 @@ solisten(struct socket *so, int backlog)
> >  	int sominconn_local = READ_ONCE(sominconn);
> >  	int error;
> >  
> > +	switch (so->so_type) {
> > +	case SOCK_STREAM:
> > +	case SOCK_SEQPACKET:
> > +		break;
> > +	default:
> > +		return (EOPNOTSUPP);
> > +	}
> > +
> >  	soassertlocked(so);
> >  
> >  	if (so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING|SS_ISDISCONNECTING))
>