Date: Sun, 21 May 2006 22:14:36 +0400 (MSD) From: Maxim Konovalov <maxim@macomnet.ru> To: Robert Watson <rwatson@FreeBSD.org> Cc: current@FreeBSD.org Subject: Re: HEADS UP: socket and pcb reference changes entering tree today Message-ID: <20060521221016.F6324@mp2.macomnet.net> In-Reply-To: <20060521190358.G8068@fledge.watson.org> References: <20060317141627.W2181@fledge.watson.org> <20060329100839.V19236@fledge.watson.org> <20060401102918.P79188@fledge.watson.org> <20060401170554.R82503@fledge.watson.org> <20060402233436.P76562@fledge.watson.org> <20060515025600.U70399@mp2.macomnet.net> <20060521185034.K8068@fledge.watson.org> <20060521215823.H6324@mp2.macomnet.net> <20060521190358.G8068@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 21 May 2006, 19:04+0100, Robert Watson wrote: > > On Sun, 21 May 2006, Maxim Konovalov wrote: > > > > 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. > > > > I "copied" this code from udp_usrreq.c::udp_disconnect(). There > > is no such lock. Is it a bug too? > > Yes. > > I have some intuitions about why the datagram protocols manually frob the > disconnected flag rather than calling soisdisconnected(), but am generally > unsure that this is the right thing. I thought about that but hadn't find any explanations. I'm testing a patch below: Index: raw_ip.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/raw_ip.c,v retrieving revision 1.161 diff -u -p -r1.161 raw_ip.c --- raw_ip.c 15 May 2006 09:28:57 -0000 1.161 +++ raw_ip.c 21 May 2006 18:13:14 -0000 @@ -671,9 +671,11 @@ rip_disconnect(struct socket *so) INP_INFO_WLOCK(&ripcbinfo); INP_LOCK(inp); inp->inp_faddr.s_addr = INADDR_ANY; + SOCK_LOCK(so); + so->so_state &= ~SS_ISCONNECTED; + SOCK_UNLOCK(so); INP_UNLOCK(inp); INP_INFO_WUNLOCK(&ripcbinfo); - so->so_state &= ~SS_ISCONNECTED; return (0); } Index: udp_usrreq.c =================================================================== RCS file: /home/ncvs/src/sys/netinet/udp_usrreq.c,v retrieving revision 1.188 diff -u -p -r1.188 udp_usrreq.c --- udp_usrreq.c 6 May 2006 11:24:59 -0000 1.188 +++ udp_usrreq.c 21 May 2006 18:12:43 -0000 @@ -1057,9 +1057,11 @@ udp_disconnect(struct socket *so) in_pcbdisconnect(inp); inp->inp_laddr.s_addr = INADDR_ANY; + SOCK_LOCK(so) + so->so_state &= ~SS_ISCONNECTED; /* XXX */ + SOCK_UNLOCK(so) INP_UNLOCK(inp); INP_INFO_WUNLOCK(&udbinfo); - so->so_state &= ~SS_ISCONNECTED; /* XXX */ return 0; } %%% -- Maxim Konovalov
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060521221016.F6324>