From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14280 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] drop unused extra char from getnameinfo() local buffer Date: Fri, 28 Jun 2019 11:13:50 -0400 Message-ID: <20190628151350.GE1506@brightrain.aerifal.cx> References: <1561683273-18265-1-git-send-email-armccurdy@gmail.com> Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="194652"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14296-gllmg-musl=m.gmane.org@lists.openwall.com Fri Jun 28 17:14:06 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1hgsZh-000oX0-Qo for gllmg-musl@m.gmane.org; Fri, 28 Jun 2019 17:14:05 +0200 Original-Received: (qmail 23659 invoked by uid 550); 28 Jun 2019 15:14:03 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 23637 invoked from network); 28 Jun 2019 15:14:02 -0000 Content-Disposition: inline In-Reply-To: <1561683273-18265-1-git-send-email-armccurdy@gmail.com> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14280 Archived-At: On Thu, Jun 27, 2019 at 05:54:33PM -0700, Andre McCurdy wrote: > The num local buffer is only passed to itoa(), which expects a buffer > size of 3*sizeof(int), not 3*sizeof(int)+1. Also change the data type > of the port local variable to clarify that itoa() only handles > unsigned values. > --- > src/network/getnameinfo.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/network/getnameinfo.c b/src/network/getnameinfo.c > index f77e73a..02c2c09 100644 > --- a/src/network/getnameinfo.c > +++ b/src/network/getnameinfo.c > @@ -124,7 +124,7 @@ int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl, > int flags) > { > char ptr[PTR_MAX]; > - char buf[256], num[3*sizeof(int)+1]; > + char buf[256], num[3*sizeof(int)]; I think the 3*sizeof(int)+1 idiom is a standard one we use throughout the code, because it's clearly valid for any size int. It's based ceil(log10(256))==3 and one byte for termination, and it would actually be sharp in the pathological case sizeof(int)==1 (which of course we don't support and can't actually be supported on a hosted implementation due to stdio constaints). In practice a constant 11 would work for known-32-bit int, but the desire here is to be obviously-safe, not to be "optimal". I think what you have found though is that the expectation in the definition of itoa is inconsistent. I probably didn't notice the inconsistency because of the *--p instead of *p. It should be either *p=0 or p+=3*sizeof(int)+1 initially, I think. Does that sound right? Either way it's harmless on the only value of sizeof(int) that actually occurs, but I'd like to fix the inconsistency here. > int af = sa->sa_family; > unsigned char *a; > unsigned scopeid; > @@ -184,7 +184,7 @@ int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl, > > if (serv && servlen) { > char *p = buf; > - int port = ntohs(((struct sockaddr_in *)sa)->sin_port); > + unsigned port = ntohs(((struct sockaddr_in *)sa)->sin_port); > buf[0] = 0; > if (!(flags & NI_NUMERICSERV)) > reverse_services(buf, port, flags & NI_DGRAM); This is ok-ish since it's consistent with the signature for itoa, but the range of value is such that it can never be negative either way. Rich