From owner-svn-src-head@freebsd.org Tue Jul 24 01:47:48 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 9FEA01031D0E; Tue, 24 Jul 2018 01:47:48 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-so.shaw.ca (smtp-out-so.shaw.ca [64.59.136.138]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id DC07577CB4; Tue, 24 Jul 2018 01:47:47 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.125.17]) by shaw.ca with ESMTPA id hmQRf3uRBwyxUhmQSfUwre; Mon, 23 Jul 2018 19:47:45 -0600 X-Authority-Analysis: v=2.3 cv=NPJhBHyg c=1 sm=1 tr=0 a=VFtTW3WuZNDh6VkGe7fA3g==:117 a=VFtTW3WuZNDh6VkGe7fA3g==:17 a=R9QF1RCXAYgA:10 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=ZG7JwcREcfMdwwf1eYsA:9 a=-7L1LNRrWOYp5KFS:21 a=wM0xPb2ybMGysFtY:21 a=wPNLvfGTeEIA:10 a=SF0I7z1-NXcR-S14nisA:9 a=j4Th2aoYb5MAmu98:21 a=OEEB9RI5AYsv71Oq:21 a=FMSec9of-L8gub9h:21 a=_W_S_7VecoQA:10 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 Received: from [25.169.253.91] (unknown [72.143.235.66]) by spqr.komquats.com (Postfix) with ESMTPSA id A948C1564; Mon, 23 Jul 2018 18:47:48 -0700 (PDT) MIME-Version: 1.0 From: Cy Schubert Subject: RE: svn commit: r336619 - head/lib/libc/gen Date: Mon, 23 Jul 2018 18:47:41 -0700 To: Ian Lepore CC: Alan Somers , src-committers , "svn-src-all@freebsd.org" , "svn-src-head@freebsd.org" Message-Id: <20180724014748.A948C1564@spqr.komquats.com> X-CMAE-Envelope: MS4wfAGUsaKfwlXU8Df61hAQQcxsOwCigFwHM4BZHRtX3ZMnQWZx/peZjcNLPEpCLQiKFPnKd11F/YcR4CNY1agM/LQZiZHGC/hAfiyd4RF/YH3ER/W0uzfq Z0E772ICwnoZhS094rUP7nlb80Ha/hZACrFuVf1Z4NkfCTtf2LMZhdzT80ciPzSygZiQgHNiRkRS/1oT7ajDebuyIqeFQFKfN8geVG7zN/NLlY3/bTY+9x9L 5LSUNzc/7XWn2zVRhgvUpgFKsC3jnmoWDm9EN4l517/3lmopK44dk/Fa4ycPfEF1P5zy0BY9YpxLLXi16PDgPRKHEJ5ZH+zgjGrTRfA1a4Y= Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.27 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 01:47:48 -0000 Well, the ssh client is still broken and IMO needs to be fixed sooner than = later. If we want to maintain compatibility right now before any rewrite is= attempted, our options are limited. Just saying that if this will be broke= n for a while we need some kind of workaround. Instead of a null pointer a = pointer to a statically defined null string. I know it's ugly and hate to s= uggest it but it gives us (you) more time for a better solution. BTW, I agree about the whack-a-mole thing given the brokenness. --- 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 17:05 To: Cy Schubert Cc: Alan Somers; src-committers; svn-src-all@freebsd.org; svn-src-head@free= bsd.org Subject: Re: svn commit: r336619 - head/lib/libc/gen 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. >=20 > --- > Sent using a tiny phone keyboard. > Apologies for any typos and autocorrect. > Also, this old phone only supports top post. Apologies. >=20 > Cy Schubert > or > The need of the many outweighs the greed of the few. > --- >=20 > -----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 >=20 > On Mon, 2018-07-23 at 13:11 -0700, Cy Schubert wrote: > >=20 > > In message <1532364679.1344.161.camel@freebsd.org>, Ian Lepore > > writes: > > >=20 > > >=20 > > > On Mon, 2018-07-23 at 09:41 -0700, Cy Schubert wrote: > > > >=20 > > > >=20 > > > > I'm sure. Rolling this libc commit back addressed the ssh > > > > segfaults > > > > on all my systems. > > > >=20 > > > > --- > > > > Sent using a tiny phone keyboard. > > > > Apologies for any typos and autocorrect. > > > > Also, this old phone only supports top post. Apologies. > > > >=20 > > > > Cy Schubert > > > > or > > > > The need of the many outweighs the greed of the few. > > > > --- > > > >=20 > > > 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 !=3D NULL should fix this > > instance. > >=20 > >=20 > No, that doesn't work, because __pw_scan just skips setting pw_class > and that leaves non-NULL garbage in that pointer. =A0There 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. >=20 > I've decided that the only safe way to fix this is to have pw_scan() > do > the same thing getpwnam() and friends do: =A0first init the pwd struct > to > known values, including supplying empty strings rather than NULL > pointers for all the char* fields, before calling __pw_scan(). >=20 > But, all of a sudden I've gotten busy with $work, so I may have to > set > all this aside for a few days. >=20 > -- Ian >=20