Re: HEADS UP: socket and pcb reference changes entering tree today

On Mon, 15 May 2006, Maxim Konovalov wrote:

> There is a bug in raw ip code processing which panics system.  I put a small 
> regression test in src/tools/regression/netinet/rawconnect.
>
> At the moment the code path for the connected raw ip socket looks like that:
>
> % soclose()
> %   sodisconnect()
> %     rip_disconnect()
> %       rip_abort()
> %         rip_pcbdetach()
> %   rip_detach <<<--------- panic
> %     rip_pcbdetach()
>
> .. and we panics in rip_detach() at KASSERT(inp != NULL).
>
> With this patch panic has gone.

This looks good in terms of pcb structure, but you should acquire SOCK_LOCK 
around the so_state manipulation.  To prevent races, I suggest doing it while 
also holding the INP lock in the center of the locking sets from the inpcb. 
There are some other remaining bugs in the raw socket code elsewhere also, I 
think.

Robert N M Watson

>
> Index: raw_ip.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.160
> diff -u -p -r1.160 raw_ip.c
> --- raw_ip.c	21 Apr 2006 09:25:39 -0000	1.160
> +++ raw_ip.c	14 May 2006 23:39:15 -0000
> _at__at_ -661,9 +661,19 _at__at_ rip_abort(struct socket *so)
> static int
> rip_disconnect(struct socket *so)
> {
> +	struct inpcb *inp;
> +
> 	if ((so->so_state & SS_ISCONNECTED) == 0)
> 		return ENOTCONN;
> -	rip_abort(so);
> +
> +	inp = sotoinpcb(so);
> +	KASSERT(inp != NULL, ("rip_disconnect: inp == NULL"));
> +	INP_INFO_WLOCK(&ripcbinfo);
> +	INP_LOCK(inp);
> +	inp->inp_faddr.s_addr = INADDR_ANY;
> +	INP_UNLOCK(inp);
> +	INP_INFO_WUNLOCK(&ripcbinfo);
> +	so->so_state &= ~SS_ISCONNECTED;
> 	return (0);
> }
> %%%
>
> -- 
> Maxim Konovalov
>