From owner-svn-src-head@FreeBSD.ORG Fri May 15 10:38:59 2009 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 85AF1106564A; Fri, 15 May 2009 10:38:59 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id CC0978FC08; Fri, 15 May 2009 10:38:58 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtps (TLSv1:AES256-SHA:256) (Exim 4.63 (FreeBSD)) (envelope-from ) id 1M4uoa-0009FN-FG; Fri, 15 May 2009 13:38:56 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id n4FAcrxR054266 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 15 May 2009 13:38:53 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3) with ESMTP id n4FAcruh066312; Fri, 15 May 2009 13:38:53 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id n4FAcrLG066311; Fri, 15 May 2009 13:38:53 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Fri, 15 May 2009 13:38:53 +0300 From: Kostik Belousov To: Peter Holm Message-ID: <20090515103853.GE1927@deviant.kiev.zoral.com.ua> References: <200905141054.n4EAsvp1088977@svn.freebsd.org> <20090515070239.GQ58540@hoeg.nl> <20090515080613.GA27593@x2.osted.lan> <20090515094852.GC1927@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="G6nVm6DDWH/FONJq" Content-Disposition: inline In-Reply-To: <20090515094852.GC1927@deviant.kiev.zoral.com.ua> User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: clamav-milter 0.95.1 at skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1M4uoa-0009FN-FG f54ff79d2976c55b1d44e7b1e917e4a9 X-Terabit: YES Cc: svn-src-head@freebsd.org, Ed Schouten , svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r192094 - head/sys/kern X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 May 2009 10:39:00 -0000 --G6nVm6DDWH/FONJq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 15, 2009 at 12:48:52PM +0300, Kostik Belousov wrote: > On Fri, May 15, 2009 at 10:06:13AM +0200, Peter Holm wrote: > > On Fri, May 15, 2009 at 09:02:39AM +0200, Ed Schouten wrote: > > > Hi Kostik, > > >=20 > > > * Konstantin Belousov wrote: > > > > Log: > > > > Do not advance req->oldidx when sysctl_old_user returning an > > > > error due to copyout failure or short buffer. > > > > =20 > > > > The later breaks the usermode iterators of the sysctl results tha= t pack > > > > arbitrary number of variable-sized structures. Iterator expects t= hat > > > > kernel filled exactly oldlen bytes, and tries to interpret half-f= illed > > > > or garbage structure at the end of the buffer. In particular, > > > > kinfo_getfile(3) segfaulted. > > > > =20 > > > > Reported and tested by: pho > > > > MFC after: 3 weeks > > >=20 > > > Is it possible that this change introduces a regression? Right now > > > `pstat -t' gets stuck in an infinite loop. I've added the following > > > printf: > > >=20 > > > | Index: pstat.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 > > > | --- pstat.c (revision 192128) > > > | +++ pstat.c (working copy) > > > | @@ -263,6 +263,7 @@ > > > | if (errno !=3D ENOMEM) > > > | err(1, "sysctlbyname()"); > > > | len *=3D 2; > > > | + printf("Going to %zu\n", len); > > > | if ((xttys =3D realloc(xttys, len)) =3D=3D NULL) > > > | err(1, "realloc()"); > > > | } > > >=20 > > > pstat on -CURRENT prints: > > >=20 > > > | LINE INQ CAN LIN LOW OUTQ USE LOW COL SESS PGID S= TATE > > > | Going to 0 > > > | Going to 0 > > > | Going to 0 > > > | ... > > >=20 > > > If I use the same patch on RELENG_6, I get the expected result: > > >=20 > > > | LINE RAW CAN OUT IHIWT ILOWT OHWT LWT COL STATE SESS = PGID DISC > > > | Going to 272 > > > | Going to 544 > > > | Going to 1088 > > > | Going to 2176 > > > | Going to 4352 > > > | Going to 8704 > > > | sysmouse 0 0 0 0 0 0 0 0 - 0 = 0 term > > > | ... > > >=20 > > > So the problem is that sysctl overwrites the len argument with 0, even > > > if it returns back to userspace with ENOMEM. > > >=20 > > > I see we have two changes in sysctl. In theory it could also be relat= ed > > > to jhb@'s changes to sysctl locking, but I suspect it's less likely. > > >=20 > >=20 > > I can confirm that it is r192094 that triggers the loop. >=20 > Yes, this is what I mean when talked about a breakage. >=20 > Below is the reversal of r192094 + the change to keep the old, ugly > behaviour of sysctl kern.proc.filedesc to return 0 on ENOMEM, but with > oldlen chopped at the end of the last completely written struct kern_info > instead of the middle of partially-written one. >=20 > Peter, could you, please, retest ? Err, the patch. diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c index f29b0eb..e0008e6 100644 --- a/sys/kern/kern_descrip.c +++ b/sys/kern/kern_descrip.c @@ -2883,6 +2883,7 @@ sysctl_kern_proc_filedesc(SYSCTL_HANDLER_ARGS) struct proc *p; struct tty *tp; int vfslocked; + size_t oldidx; =20 name =3D (int *)arg1; if ((p =3D pfind((pid_t)name[0])) =3D=3D NULL) @@ -3061,14 +3062,26 @@ sysctl_kern_proc_filedesc(SYSCTL_HANDLER_ARGS) strlen(kif->kf_path) + 1; kif->kf_structsize =3D roundup(kif->kf_structsize, sizeof(uint64_t)); + oldidx =3D req->oldidx; error =3D SYSCTL_OUT(req, kif, kif->kf_structsize); - if (error) + if (error) { + if (error =3D=3D ENOMEM) { + /* + * The hack to keep the ABI of sysctl + * kern.proc.filedesc intact, but not + * to account a partially copied + * kinfo_file into the oldidx. + */ + req->oldidx =3D oldidx; + error =3D 0; + } break; + } } FILEDESC_SUNLOCK(fdp); fddrop(fdp); free(kif, M_TEMP); - return (0); + return (error); } =20 static SYSCTL_NODE(_kern_proc, KERN_PROC_FILEDESC, filedesc, CTLFLAG_RD, diff --git a/sys/kern/kern_sysctl.c b/sys/kern/kern_sysctl.c index bf539be..0a8a096 100644 --- a/sys/kern/kern_sysctl.c +++ b/sys/kern/kern_sysctl.c @@ -1223,9 +1223,9 @@ sysctl_old_kernel(struct sysctl_req *req, const void = *p, size_t l) if (i > 0) bcopy(p, (char *)req->oldptr + req->oldidx, i); } + req->oldidx +=3D l; if (req->oldptr && i !=3D l) return (ENOMEM); - req->oldidx +=3D l; return (0); } =20 @@ -1322,10 +1322,9 @@ sysctl_old_user(struct sysctl_req *req, const void *= p, size_t l) size_t i, len, origidx; =20 origidx =3D req->oldidx; - if (req->oldptr =3D=3D NULL) { - req->oldidx +=3D l; + req->oldidx +=3D l; + if (req->oldptr =3D=3D NULL) return (0); - } /* * If we have not wired the user supplied buffer and we are currently * holding locks, drop a witness warning, as it's possible that @@ -1347,7 +1346,6 @@ sysctl_old_user(struct sysctl_req *req, const void *p= , size_t l) return (error); if (i < l) return (ENOMEM); - req->oldidx +=3D l; return (0); } =20 --G6nVm6DDWH/FONJq Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkoNRjwACgkQC3+MBN1Mb4iNVwCgxEsXB7aKNdZq+YnSK30UT0C6 24AAn1IaZdSk7cbb5fn9TUp+N2WIhqnt =vaye -----END PGP SIGNATURE----- --G6nVm6DDWH/FONJq--