setsockopt(2) bug.

[Steve Cumming]

Here's an interesting little glitch in the 4.3BSD socket handling code - 
specifically in the toggling of socket level options.

Just in case this has not been noticed before, here's a description,
and a fix.

What happens is that setsockopt(3) returns with EINVAL
whenever a socket level boolean option is reset.

Here's how to duplicate it:

------------------------ Cut Here ------------------------------


/* I may have forgotten a #include or two....
 
	This code comes from tftpd.c

*/

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/tftp.h>
#include <stdio.h>
#include <errno.h>
#include <ctype.h>

extern	int errno;
struct	sockaddr_in sin = { AF_INET };
int	f;

main(argc, argv)
	char *argv[];
{
	register int n,f;
	struct servent *sp;


	sp = getservbyname("tftp", "udp");
	if (sp == 0) {
		exit(1);
	}
	sin.sin_port = sp->s_port;

	f = socket(AF_INET, SOCK_DGRAM, 0);
	if (f < 0) {
		exit(0);
	}

	/* BREAKS HERE		*/
	/* Returns EINVAL	*/

	if (setsockopt(f, SOL_SOCKET, SO_REUSEADDR, (char *)0, sizeof(int)) < 0) {
		perror("tftpd: setsockopt (SO_REUSEADDR)");
	}
	(void) close(f);
}



----------------------------- Cut Here -------------------------------

Now for the bug.

I quote from the manual entry for {get|set}sockopt(2):

	setsockopt(s,level,optname,optval,optlen)
	int s,level,optname;
	char *optval;
	int optlen;

	...To manipulate options at the "socket" level,
	level is specified as SOL_SOCKET. 

	The parameters optval and optlen are used to access option values
	for setsockopt. ... If not option value is to be supplied or
	returned, optval may be suppled as 0.

	Most socket level options take an int parameter for optval.
	For setsockopt, the parameter should [be] non-zero to enable a boolean
	option, or zero if the option is be disabled.
	[SO_REUSEADDR is such an option]

Now this is a classic example of overdriving innocent arguments.
Here's what really happens:

First, look at the actual code in uipc_syscalls.c...


setsockopt()
{
	struct a {
		int	s;
		int	level;
		int	name;
		caddr_t	val;
		int	valsize;
	} *uap = (struct a *)u.u_ap;
	struct file *fp;
	struct mbuf *m = NULL;

	fp = getsock(uap->s);
	if (fp == 0)
		return;
	if (uap->valsize > MLEN) {
		u.u_error = EINVAL;
		return;
	}

/* SFU DEBUG

	On those occaisions when val is supposesd to
	be 0 (i.e. resetting many socket level flags)
	we lose. mbuf pointer m stays NULL, causing
	sosetopt to return EINVAL.
 

	if (uap->val) {

  END SFU DEBUG
*/ 	
	if (uap->val >= 0){	
			
		get an mbuf 'm', copyin 'uap->valsize' bytes
		from where 'uap->val' points to;
		m->m_len = uap->valsize;
	}
	u.u_error =
	    sosetopt((struct socket *)fp->f_data, uap->level, uap->name, m);
}
	
Now we go to uipc_socket.c, where sosetopt() lives:
Unfortunately, it wants the arguments nicely packaged 
in an mbuf. But the distributed version doesn't build
one if the option value is 0.
	

sosetopt(so, level, optname, m0)
	register struct socket *so;
	int level, optname;
	struct mbuf *m0;
{
	int error = 0;
	register struct mbuf *m = m0;

	if (level != SOL_SOCKET) {
		if (so->so_proto && so->so_proto->pr_ctloutput)
			return ((*so->so_proto->pr_ctloutput)
				  (PRCO_SETOPT, so, level, optname, &m0));
		error = ENOPROTOOPT;
	} else {
		switch (optname) {

		case SO_REUSEADDR: (and other socket level toggles)


/* SFU COMMENT.
	This is the wrong test, maybe.
	As distributed, 'm' is null whenever
	a socket level toggle is being reset.
	Unfortunately, this causes an (utterly mysterious)
	EINVAL, as you can see.
*/	
			if (m == NULL || m->m_len < sizeof (int)) {
				error = EINVAL;
				goto bad;
			}
			if (*mtod(m, int *))
				so->so_options |= optname;
			else
				so->so_options &= ~optname;
			break;
/*
	forget this part
*/

		}
	}
bad:
	if (m)
		(void) m_free(m);
	return (error);
}



My fix, which appears to work correctly, is to change the test in
setsockopt() so that an mbuf is always built. (As shown above)

It would probably be better if setsockopt()
did its test on optlen rather than
optval, encoding toggle values when optlen is zero.

soseopt() could then dispense with the sanity check on
its'  mbuf arument, which seems superfluous anyway.
Then (mbuf *)m  could be coerced into a (int *)
and the option toggled accordingly. This gets around the overhead
of generating an mbuf, which I guess was the point in the first 
place. Of course to do it right, sosetopt() would require
an extra parameter.



Steve Cumming

School of Computing Science
Simon Fraser University
Burnaby, B.C.
Canada

...ubc-cs!fornax!stevec
steve at lccr.sfu.cdn