Date: Wed, 31 Dec 2003 17:30:28 +0200 From: Peter Pentchev <roam@ringlet.net> To: Chris McKenzie <cjmckenzie@ucdavis.edu> Cc: Kris Kennaway <kris@obsecurity.org> Subject: Re: How to hard lock FreeBSD-5.1 generic with sl Message-ID: <20031231153028.GA901@straylight.m.ringlet.net> In-Reply-To: <20031230142800.GA707@straylight.m.ringlet.net> References: <Pine.GSO.4.44.0312291800260.8920-100000@veni.ucdavis.edu> <20031230141253.GA40702@xor.obsecurity.org> <20031230142800.GA707@straylight.m.ringlet.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031231153028.GA901>