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: 572220 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id AA237140BAB for ; Sun, 24 Jan 2016 12:48:54 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=SGiL4Hdf; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type; q=dns; s=default; b=kY62 WYwSwLSUzL80UKyNNVPdUaC09hbrZfpOHhtWIjTlDM3TCR8+Cro0ru4vWPRfrSBA RdCPlQr4m7KROxLWNW3f9JTWK+/T/die4MJA+4ke5VAJXsub/PMiCr8CSzQqgT6R QPVg8/yIAB4MK4aIJ1zuGLVzneQyrd5SGjVkUD4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:from:message-id:date :mime-version:in-reply-to:content-type; s=default; bh=Mf2nJMQc+N qrMDGrZgc4fkOlsOE=; b=SGiL4Hdf9utlZca4tsLT2y7HUxnp07wVYg2vzcT7bW va1wFqdtkcFk4hugiiQW6cWc6oHZTmPuEPyqM1z3Fu2LiL6vLbonROz+m1th1Qmz dDP+rCFSxQS/Mm5yNSVsrRmUkDcOVeWtT5DdB09LsRtxq6tavhNsoRiXv5ozFCMk M= 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;