From patchwork Sun Jan 24 01:48:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 10537 Received: (qmail 78221 invoked by alias); 24 Jan 2016 01:48:47 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 77345 invoked by uid 89); 24 Jan 2016 01:48:44 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=71, 6, multiplication, Hx-languages-length:1784, prime X-HELO: zimbra.cs.ucla.edu Subject: Re: [PATCH] Improve check against integer wraparound in hcreate_r [BZ #18240] To: Florian Weimer , GNU C Library , Adhemerval Zanella References: <56A210C4.80609@redhat.com> From: Paul Eggert Message-ID: <56A42D78.1030506@cs.ucla.edu> Date: Sat, 23 Jan 2016 17:48:40 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56A210C4.80609@redhat.com> Florian Weimer wrote: > - if (nel >= SIZE_MAX / sizeof (_ENTRY)) > + /* This limit is sufficient to avoid unsigned wraparound below, > + possibly after truncation to unsigned int. (struct hsearch_data > + is part of the public API and uses usigned ints.) */ > + if (nel >= INT_MAX / sizeof (_ENTRY)) This patch doesn't look right. nel should be bounded by UINT_MAX - 2, not by INT_MAX / sizeof (anything). (Not by UINT_MAX, since the code computes nel + 1 later; and not by UINT_MAX - 1 since that cannot be prime.) Furthermore, calloc will check for size overflow on multiplication so hcreate_r need not worry about dividing by sizeof (anything). Also, "unsigned" is misspelled in the comment. How about something like the attached (untested) patch instead? diff --git a/misc/hsearch_r.c b/misc/hsearch_r.c index f6f16ed..c258397 100644 --- a/misc/hsearch_r.c +++ b/misc/hsearch_r.c @@ -71,13 +71,6 @@ __hcreate_r (size_t nel, struct hsearch_data *htab) return 0; } - if (nel >= SIZE_MAX / sizeof (_ENTRY)) - { - __set_errno (ENOMEM); - return 0; - } - - /* There is still another table active. Return with error. */ if (htab->table != NULL) return 0; @@ -86,10 +79,19 @@ __hcreate_r (size_t nel, struct hsearch_data *htab) use will not work. */ if (nel < 3) nel = 3; - /* Change nel to the first prime number not smaller as nel. */ - nel |= 1; /* make odd */ - while (!isprime (nel)) - nel += 2; + + /* Change nel to the first prime number in the range [nel, UINT_MAX - 2], + The '- 2' means 'nel += 2' cannot overflow. */ + for (nel |= 1; ; nel += 2) + { + if (UINT_MAX - 2 < nel) + { + __set_errno (ENOMEM); + return 0; + } + if (isprime (nel)) + break; + } htab->size = nel; htab->filled = 0;