"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: batch up tree entries in imsg
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Wed, 18 May 2022 19:46:14 +0200

Download raw body.

Thread
On Wed, May 18, 2022 at 06:29:33PM +0200, Omar Polo wrote:
> > > I'm not sure I'm getting the meaning of this check:
> > > 
> > > > +				if (sizeof(ite) + ite.namelen < sizeof(ite)) {
> > > 
> > > ite.nameles is size_t so how can this evaluate to true?
> > 
> > This checks for unsigned integer overflow into the range [0, sizeof(ite)-1].
> > Remember that libexec code can send us garbage; the value of ite.namelen
> > comes from libexec code. Perhaps other checks already cover this, but the
> > more the better I guess.
> 
> I see, haven't thought about the unsigned integer overflow.  I'm still
> not sure that's needed, it's an expression involving only `size_t's and
> reached that point we already know that namelen is less than NAME_MAX+1
> (sizeof te->name).  But as you said it's probably better to keep these
> checks even if slightly redundant.

I added the sizeof(te->name) check later while writing this code.
The sizeof(te->name) check indeed cuts down the possible range considerably.
So I'll drop the above overflow check. I suspect the compiler would drop
this extra check anyway on grounds of "undefined behaviour" or "cannot
happen", even if we left it in place.