From owner-svn-src-head@freebsd.org Tue Jul 24 00:05:30 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 192EC1057AD1 for ; Tue, 24 Jul 2018 00:05:30 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1b.ore.mailhop.org (outbound1b.ore.mailhop.org [54.200.247.200]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 9602073CCB for ; Tue, 24 Jul 2018 00:05:29 +0000 (UTC) (envelope-from ian@freebsd.org) X-MHO-RoutePath: aGlwcGll X-MHO-User: 449126fd-8ed5-11e8-93fa-f3ebd9db2b94 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound1.ore.mailhop.org (Halon) with ESMTPSA id 449126fd-8ed5-11e8-93fa-f3ebd9db2b94; Tue, 24 Jul 2018 00:05:27 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id w6O05QQs014151; Mon, 23 Jul 2018 18:05:26 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <1532390726.1344.194.camel@freebsd.org> Subject: Re: svn commit: r336619 - head/lib/libc/gen From: Ian Lepore To: Cy Schubert Cc: Alan Somers , src-committers , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Date: Mon, 23 Jul 2018 18:05:26 -0600 In-Reply-To: <20180723235258.2DB5EC73@spqr.komquats.com> References: <20180723235258.2DB5EC73@spqr.komquats.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.18.5.1 FreeBSD GNOME Team Port Mime-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 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: Tue, 24 Jul 2018 00:05:30 -0000 I don't wanna play whack-a-mole like that. This whole area is a great big mess, and it needs a great big cleanup, and I've got too many other irons in the fire right now to do that. -- Ian On Mon, 2018-07-23 at 16:52 -0700, Cy Schubert wrote: > I think you misunderstood me because of my terse email. Sorry about > that. We can address this with a NULL check in openssh's misc.c. I > can't recall the actual path. I'll post a patch, which will explain > it better than I can in English, as soon as I get home. > > --- > Sent using a tiny phone keyboard. > Apologies for any typos and autocorrect. > Also, this old phone only supports top post. Apologies. > > Cy Schubert > or > The need of the many outweighs the greed of the few. > --- > > -----Original Message----- > From: Ian Lepore > Sent: 23/07/2018 13:40 > To: Cy Schubert > Cc: Alan Somers; src-committers; svn-src-all@freebsd.org; svn-src-hea > d@freebsd.org > Subject: Re: svn commit: r336619 - head/lib/libc/gen > > On Mon, 2018-07-23 at 13:11 -0700, Cy Schubert wrote: > > > > In message <1532364679.1344.161.camel@freebsd.org>, Ian Lepore > > writes: > > > > > > > > > On Mon, 2018-07-23 at 09:41 -0700, Cy Schubert wrote: > > > > > > > > > > > > I'm sure. Rolling this libc commit back addressed the ssh > > > > segfaults > > > > on all my systems. > > > > > > > > --- > > > > Sent using a tiny phone keyboard. > > > > Apologies for any typos and autocorrect. > > > > Also, this old phone only supports top post. Apologies. > > > > > > > > Cy Schubert > > > > or > > > > The need of the many outweighs the greed of the few. > > > > --- > > > > > > > My current working theory is that some of the software that uses > > > __pw_scan() pre-stages a pointer-to-empty-string into the > > > pw_class > > > field and my change ruined that by replacing it with a NULL > > > pointer. > > > Other callers of __pw_scan() don't do that, they just assume > > > they're > > > running as root and will get all the fields populated. > > Yes. A simple check for pw->pw_class != NULL should fix this > > instance. > > > > > No, that doesn't work, because __pw_scan just skips setting pw_class > and that leaves non-NULL garbage in that pointer.  There is a > pw_fields > field that uses bits to indicate which fields were parsed, but almost > nothing uses it, so I'm reluctant to rely on it because this same > kind > of unexpected crash of some random tool will happen if there's > anything > that fills out a pwd struct without making those flags valid then > calls > pw_copy() which would rely on them being valid. > > I've decided that the only safe way to fix this is to have pw_scan() > do > the same thing getpwnam() and friends do:  first init the pwd struct > to > known values, including supplying empty strings rather than NULL > pointers for all the char* fields, before calling __pw_scan(). > > But, all of a sudden I've gotten busy with $work, so I may have to > set > all this aside for a few days. > > -- Ian >