From patchwork Tue Oct 27 02:04:43 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 536421 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 9D0FE140E31 for ; Tue, 27 Oct 2015 13:05:25 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=XD0M0CEj; 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:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; q=dns; s=default; b=Sb4P dlXhfmVBe+0KVudarbdrkMlmrBv3VBWye/XgzFiKVSp6NNSaEQHJfdV83tXPsMyQ vglvGzNk/IZIJ9sfDuW9aR9oN1xh/HObKkSErsfZXpGQ3sHfo1DoxV1bqnisq8Gl ZN3DMFQWUE7k328ERgpUrKrA9e3LwA6J3IwhwtA= 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:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; s=default; bh=5xhFUl1p5o T5OJLK23gCnpG20cg=; b=XD0M0CEj7YBnM7nlInH1nNhZIKR2eXt2xOM/FnRI3S wrsnxAThXvfn5kakgjO4h7GdwxWsjYDgpoZcY3E6KNb2Jw6EVf8pSVlXSR2Ko/wf mqr4V4PDVtPEBj0x65xLi3YoDMeieVEQZtomugY/oP3sC34H24cxdV3yKlImZcl1 Q= Received: (qmail 89771 invoked by alias); 27 Oct 2015 02:05:18 -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 89759 invoked by uid 89); 27 Oct 2015 02:05:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.0 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS, URIBL_BLACK, URIBL_JP_SURBL autolearn=no version=3.3.2 X-HELO: mail-wi0-f182.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=lKtqqoiUljVHVCmOT4cyQIi0AChpXKkdsIOh0jX+86U=; b=h+Rlk6RVCd9xgCLB0J+MH8jrY4+mIFzdJ+6NmwUvAAe0OITtI9GV42T3fMj38twB5m P8JNyu1aAlmlT52bVMSg5H1jQ69SFpH3hg16OQbskr/DqWoICuQLEh33EIjM8TGXS5xC FBnLuv7y2MGIRdI6pNY5aLCh4NYttTMPVZqRzo8HV8+y9q2vohUXVnzjyGEB5YOmRCj7 X2B3YU4ARe69O89HlUR4LK1yyk3k2indVv1F3lK6S5e9kfGTYym2g2jY+LQiYCH2HxC3 glAjYC2O2O12E7Z/LF7b3uIH7eRKHwRZMw/gEQrQiCqTvkHAwMQaTu4RHzKUouE6sclp czIw== X-Gm-Message-State: ALoCoQlc7ca77yneC9Af3u0NWwpODybQI9z1w/AJS14IOrZscfSORLiGp5sPCiURr8pnqeF6f2Em X-Received: by 10.194.62.112 with SMTP id x16mr50894579wjr.132.1445911513324; Mon, 26 Oct 2015 19:05:13 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20151026200605.GI8645@brightrain.aerifal.cx> References: <20151026200605.GI8645@brightrain.aerifal.cx> From: Paul Pluzhnikov Date: Mon, 26 Oct 2015 19:04:43 -0700 Message-ID: Subject: Re: [patch] Fix BZ 19165 -- overflow in fread / fwrite To: Rich Felker Cc: GLIBC Devel , Alexander Cherepanov , Florian Weimer , "Joseph S. Myers" On Mon, Oct 26, 2015 at 1:06 PM, Rich Felker wrote: > This highly pessimizes short reads/writes, e.g. fwrite(&c,1,1,f), by > introducing a div operation. The obvious intent of the original code > was to avoid this. Thanks for comments, everyone. Revised patch addresses all of them (I believe). Not sure whether the name or location of the saturating multiplication is acceptable though. Suggestions welcome. 2015-10-26 Paul Pluzhnikov [BZ #19165] * libio/libioP.h (_IO_saturating_umull): New. * libio/iofread.c (_IO_fread): Use it. * ibio/iofread_u.c (__fread_unlocked): Likewise. * libio/iofwrite.c (_IO_fwrite): Error on overflow. * libio/iofwrite_u.c (fwrite_unlocked): Likewise. diff --git a/libio/iofread.c b/libio/iofread.c index eb69b05..d2a42bc 100644 --- a/libio/iofread.c +++ b/libio/iofread.c @@ -29,7 +29,7 @@ _IO_size_t _IO_fread (void *buf, _IO_size_t size, _IO_size_t count, _IO_FILE *fp) { - _IO_size_t bytes_requested = size * count; + _IO_size_t bytes_requested = _IO_saturating_umull (size, count); _IO_size_t bytes_read; CHECK_FILE (fp, 0); if (bytes_requested == 0) diff --git a/libio/iofread_u.c b/libio/iofread_u.c index 997b714..8c3a821 100644 --- a/libio/iofread_u.c +++ b/libio/iofread_u.c @@ -32,7 +32,7 @@ _IO_size_t __fread_unlocked (void *buf, _IO_size_t size, _IO_size_t count, _IO_FILE *fp) { - _IO_size_t bytes_requested = size * count; + _IO_size_t bytes_requested = _IO_saturating_umull (size, count); _IO_size_t bytes_read; CHECK_FILE (fp, 0); if (bytes_requested == 0) diff --git a/libio/iofwrite.c b/libio/iofwrite.c index 48ad4bc..9bce404 100644 --- a/libio/iofwrite.c +++ b/libio/iofwrite.c @@ -29,7 +29,7 @@ _IO_size_t _IO_fwrite (const void *buf, _IO_size_t size, _IO_size_t count, _IO_FILE *fp) { - _IO_size_t request = size * count; + _IO_size_t request = _IO_saturating_umull (size, count); _IO_size_t written = 0; CHECK_FILE (fp, 0); if (request == 0) diff --git a/libio/iofwrite_u.c b/libio/iofwrite_u.c index 2b1c47a..081a7b1 100644 --- a/libio/iofwrite_u.c +++ b/libio/iofwrite_u.c @@ -33,7 +33,7 @@ _IO_size_t fwrite_unlocked (const void *buf, _IO_size_t size, _IO_size_t count, _IO_FILE *fp) { - _IO_size_t request = size * count; + _IO_size_t request = _IO_saturating_umull (size, count); _IO_size_t written = 0; CHECK_FILE (fp, 0); if (request == 0) diff --git a/libio/libioP.h b/libio/libioP.h index b1ca774..40dd9b8 100644 --- a/libio/libioP.h +++ b/libio/libioP.h @@ -890,3 +890,24 @@ _IO_acquire_lock_clear_flags2_fct (_IO_FILE **p) | _IO_FLAGS2_SCANF_STD); \ } while (0) #endif + +/* Returns a*b if the result doesn't overflow, else SIZE_MAX. */ +static inline size_t +__attribute__ ((__always_inline__)) +_IO_saturating_umull (size_t a, size_t b) +{ +#if __GNUC_PREREQ(5, 0) + size_t result; + + if (__builtin_umull_overflow (a, b, &result)) { + return SIZE_MAX; + } + return result; +#else + const size_t mul_no_overflow = (size_t) 1 << 4 * sizeof (size_t); + if ((a >= mul_no_overflow || b >= mul_no_overflow) + && b > 1 && a > SIZE_MAX / b) + return SIZE_MAX; + return a * b; +#endif +}