Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190219223046.GY23599@brightrain.aerifal.cx>
Date: Tue, 19 Feb 2019 17:30:46 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Error in getaddrinfo()?

On Tue, Feb 19, 2019 at 01:09:21PM -0800, Michael Forney wrote:
> On 2019-02-19, Markus Wichmann <nullplan@....net> wrote:
> > And while we're on the subject, a few lines later we get
> >
> > 			.ai_next = &out[k+1].ai };
> >
> > Now, for the last k, isn't this calculation undefined? The array index
> > is out of bounds, then. It won't matter what is calculated here, since
> > the last .ai_next is explicitly nulled a few lines further down, but the
> > calculation might invoke undefined behavior, and these last few years
> > compilers have gotten really agressive about that.
> 
> I don't think it is undefined behavior, as long as it is not
> dereferenced. See http://port70.net/~nsz/c/c11/n1570.html#6.5.6p8:
> 
> "If both the pointer operand and the result point to elements of the
> same array object, or one past the last element of the array object,
> the evaluation shall not produce an overflow; otherwise, the behavior
> is undefined. If the result points one past the last element of the
> array object, it shall not be used as the operand of a unary *
> operator that is evaluated."

This would be true if not for the ".ai". As written, I think it may be
UB, but it's questionable whether that depends on how much extra space
was allocated for ai_canonname. In any case it's bad. The initializer
should probably be 0, followed by:

	if (k) out[k-1].ai.ai_next = &out[k].ai;

That would get rid of the need to later zero the final one, too.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.