Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 23 Jul 2018 18:05:26 -0600
From:      Ian Lepore <ian@freebsd.org>
To:        Cy Schubert <Cy.Schubert@cschubert.com>
Cc:        Alan Somers <asomers@freebsd.org>, src-committers <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r336619 - head/lib/libc/gen
Message-ID:  <1532390726.1344.194.camel@freebsd.org>
In-Reply-To: <20180723235258.2DB5EC73@spqr.komquats.com>
References:  <20180723235258.2DB5EC73@spqr.komquats.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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
> <Cy.Schubert@cschubert.com> or <cy@freebsd.org>
> 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
> > > > <Cy.Schubert@cschubert.com> or <cy@freebsd.org>
> > > > 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
> 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1532390726.1344.194.camel>