From owner-freebsd-current@FreeBSD.ORG Sun May 21 18:14:38 2006 Return-Path: X-Original-To: current@FreeBSD.org Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id B3DDB16A481; Sun, 21 May 2006 18:14:38 +0000 (UTC) (envelope-from maxim@macomnet.ru) Received: from mp2.macomnet.net (mp2.macomnet.net [195.128.64.6]) by mx1.FreeBSD.org (Postfix) with ESMTP id 13CAA43D46; Sun, 21 May 2006 18:14:37 +0000 (GMT) (envelope-from maxim@macomnet.ru) Received: from localhost (localhost [127.0.0.1]) by mp2.macomnet.net (8.13.4/8.13.3) with ESMTP id k4LIEanh011803; Sun, 21 May 2006 22:14:37 +0400 (MSD) (envelope-from maxim@macomnet.ru) Date: Sun, 21 May 2006 22:14:36 +0400 (MSD) From: Maxim Konovalov To: Robert Watson In-Reply-To: <20060521190358.G8068@fledge.watson.org> Message-ID: <20060521221016.F6324@mp2.macomnet.net> 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> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Cc: current@FreeBSD.org Subject: Re: HEADS UP: socket and pcb reference changes entering tree today X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 21 May 2006 18:14:43 -0000 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