From owner-freebsd-net@FreeBSD.ORG Wed Dec 31 07:29:37 2003 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2032616A4CE for ; Wed, 31 Dec 2003 07:29:37 -0800 (PST) Received: from gandalf.online.bg (gandalf.online.bg [217.75.128.9]) by mx1.FreeBSD.org (Postfix) with SMTP id EE37D43D53 for ; Wed, 31 Dec 2003 07:29:28 -0800 (PST) (envelope-from roam@ringlet.net) Received: (qmail 28156 invoked from network); 31 Dec 2003 15:27:27 -0000 Received: from office.casyst.com (HELO straylight.m.ringlet.net) (212.91.166.145) by gandalf.online.bg with SMTP; 31 Dec 2003 15:27:27 -0000 Received: (qmail 1175 invoked by uid 1000); 31 Dec 2003 15:30:28 -0000 Date: Wed, 31 Dec 2003 17:30:28 +0200 From: Peter Pentchev To: Chris McKenzie Message-ID: <20031231153028.GA901@straylight.m.ringlet.net> Mail-Followup-To: Chris McKenzie , Kris Kennaway , freebsd-bugs@FreeBSD.org, freebsd-net@FreeBSD.org References: <20031230141253.GA40702@xor.obsecurity.org> <20031230142800.GA707@straylight.m.ringlet.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="2B/JsCI69OhZNC5r" Content-Disposition: inline In-Reply-To: <20031230142800.GA707@straylight.m.ringlet.net> User-Agent: Mutt/1.5.5.1i cc: freebsd-net@FreeBSD.org cc: freebsd-bugs@FreeBSD.org cc: Kris Kennaway Subject: Re: How to hard lock FreeBSD-5.1 generic with sl X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 31 Dec 2003 15:29:37 -0000 --2B/JsCI69OhZNC5r Content-Type: multipart/mixed; boundary="AhhlLboLdkugWU4S" Content-Disposition: inline --AhhlLboLdkugWU4S Content-Type: text/plain; charset=windows-1251 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 30, 2003 at 04:28:00PM +0200, Peter Pentchev wrote: > On Tue, Dec 30, 2003 at 06:12:53AM -0800, Kris Kennaway wrote: > > On Mon, Dec 29, 2003 at 06:02:45PM -0800, Chris McKenzie wrote: > > > On three machines (PII 450, P3 450, Pentium laptop 200) with FreeBSD-= 5.1 > > > generic (and specific builds) I am able to completely hard lock the s= ystem > > > by doing the following > > >=20 > > > # ifconfig ppp0 create > > > # ifconfig sl0 create > > >=20 > > > Heh . . . that shouldn't happen. > >=20 > > Does the problem persist with 5.2? >=20 > I just tested in on a 5.2-CURRENT as of today, and yes, the system > locked up solid - no ddb, no anything. I'll try to do some more testing > as time permits. [cc'd to -net for a pre-commit review / discussion] OK, I think I've found the problem. The if_clone_attach() routine in src/sys/net/if.c blindly adds the new cloned interface to the if_cloners list without checking if it is already on the list. This, understandably, leads to problems when trying to attach an interface that already exists - such as a ppp interface. The if_ppp code adds itself to the if_cloners list at the module loading stage. Thus, the very first invocation of ifconfig ppp0 create adds the ppp_cloner structure to the list *again* - and creates a loop on the list. Any attempts to traverse the list later lead to lock-ups. Attached is a patch that does two things: first, only adds the interface to the list if it is not already there (the second and third chunks, at lines 812 and 827 of if.c), and second, adds a if_check_cloners_loop() routine to traverse the if_cloners list and panic if a loop is found. The if_check_cloners_loop() invocations could be protected by INVARIANTS, KASSERT, or WITNESS, but it sure helps find such problems :) Chris, could you try this patch and see if it helps in your situation? And.. happy New Year, everyone! (albeit a little early :) G'luck, Peter --=20 Peter Pentchev roam@ringlet.net roam@sbnd.net roam@FreeBSD.org PGP key: http://people.FreeBSD.org/~roam/roam.key.asc Key fingerprint FDBA FD79 C26F 3C51 C95E DF9E ED18 B68D 1619 4553 I am not the subject of this sentence. --AhhlLboLdkugWU4S Content-Type: text/plain; charset=windows-1251 Content-Disposition: attachment; filename="cloners-loop.patch" Content-Transfer-Encoding: quoted-printable Index: src/sys/net/if.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /home/ncvs/src/sys/net/if.c,v retrieving revision 1.174 diff -u -r1.174 if.c --- src/sys/net/if.c 26 Dec 2003 18:09:35 -0000 1.174 +++ src/sys/net/if.c 31 Dec 2003 15:15:25 -0000 @@ -762,6 +762,32 @@ } =20 /* + * Check the if_cloners list for loops. + */ +static void +if_check_cloners_loop(void) +{ + struct if_clone *ifc, *ifcn, *ifct; + + for (ifc =3D LIST_FIRST(&if_cloners); ifc !=3D NULL; ) { + ifcn =3D LIST_NEXT(ifc, ifc_list); + if (ifcn =3D=3D NULL) + return; + if (ifcn =3D=3D ifc) + panic( + "cloners loop to self for %p / %s", + ifc, ifc->ifc_name); + for (ifct =3D LIST_FIRST(&if_cloners); ifct !=3D ifc; + ifct =3D LIST_NEXT(ifct, ifc_list)) + if (ifct =3D=3D ifcn) + panic( + "cloners loop from %p / %s to %p / %s", + ifc, ifc->ifc_name, ifct, ifct->ifc_name); + ifc =3D ifcn; + } +} + +/* * Look up a network interface cloner. */ static struct if_clone * @@ -771,6 +797,7 @@ const char *cp; int i; =20 + if_check_cloners_loop(); for (ifc =3D LIST_FIRST(&if_cloners); ifc !=3D NULL;) { for (cp =3D name, i =3D 0; i < ifc->ifc_namelen; i++, cp++) { if (ifc->ifc_name[i] !=3D *cp) @@ -812,6 +839,8 @@ int err; int len, maxclone; int unit; + int found; + struct if_clone *ift; =20 KASSERT(ifc->ifc_minifs - 1 <=3D ifc->ifc_maxunit, ("%s: %s requested more units then allowed (%d > %d)", @@ -827,8 +856,19 @@ ifc->ifc_units =3D malloc(len, M_CLONE, M_WAITOK | M_ZERO); ifc->ifc_bmlen =3D len; =20 - LIST_INSERT_HEAD(&if_cloners, ifc, ifc_list); - if_cloners_count++; + if_check_cloners_loop(); + found =3D 0; + LIST_FOREACH(ift, &if_cloners, ifc_list) { + if (ift =3D=3D ifc) { + found =3D 1; + break; + } + } + if (!found) { + LIST_INSERT_HEAD(&if_cloners, ifc, ifc_list); + if_cloners_count++; + if_check_cloners_loop(); + } =20 for (unit =3D 0; unit < ifc->ifc_minifs; unit++) { err =3D (*ifc->ifc_create)(ifc, unit); @@ -840,7 +880,9 @@ bytoff =3D unit >> 3; bitoff =3D unit - (bytoff << 3); ifc->ifc_units[bytoff] |=3D (1 << bitoff); + if_check_cloners_loop(); } + if_check_cloners_loop(); } =20 /* @@ -853,6 +895,7 @@ LIST_REMOVE(ifc, ifc_list); free(ifc->ifc_units, M_CLONE); if_cloners_count--; + if_check_cloners_loop(); } =20 /* @@ -877,6 +920,7 @@ count =3D (if_cloners_count < ifcr->ifcr_count) ? if_cloners_count : ifcr->ifcr_count; =20 + if_check_cloners_loop(); for (ifc =3D LIST_FIRST(&if_cloners); ifc !=3D NULL && count !=3D 0; ifc =3D LIST_NEXT(ifc, ifc_list), count--, dst +=3D IFNAMSIZ) { strlcpy(outbuf, ifc->ifc_name, IFNAMSIZ); --AhhlLboLdkugWU4S-- --2B/JsCI69OhZNC5r Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.4 (FreeBSD) iD8DBQE/8uuU7Ri2jRYZRVMRAjmnAKCBRoQ0ppxBgaLLtRqhQAPiXROOBgCgjBus DMWqmZtFMV+xJ6ysUMut2cs= =j7V8 -----END PGP SIGNATURE----- --2B/JsCI69OhZNC5r--