From patchwork Mon Dec 7 17:21:24 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 9919 Received: (qmail 21199 invoked by alias); 7 Dec 2015 17:22:04 -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 21116 invoked by uid 89); 7 Dec 2015 17:22:03 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-wm0-f50.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=S/iFz5EXf5CmZPrdpqLbBmn9MdQR9qfBgsv7PsWrFjM=; b=TVnwVjqTD4hqlyxBugbSr0UcWmFRMOTe4Nut5v534F41SNUQSXu8FxHu7Rv8btz+5e ePFXdhgcloX1g+TbHZf6Dk8ePhdprgiEmj818o09k29fDlW76xt143ib10d5J0dA02M4 XJwU7DXOTlpt7+JHvhQYCnZdq60lFRCAd2CBb43NlOne1RNaIauPHPJ9hHr7yC5cZReS QzYQdiONKR9M3YA8F4tjabV3x96h6GgO4rQ+9JgouXL6e7D3Ji2iAhf92EhpAohaGC5P PoMVSiHunadt1zxVfJqtjqV1RcWJvScPiRhJPRTrkzY1Afojl4iV+UcbfJ925UT4FUKy /5YA== X-Gm-Message-State: ALoCoQnvYUlj/o1UCVy4tK9DG5vGqnM6PUwI/6qUodg5lWSjEGu8rMg7dr0SOGEiDi5BnZUHBHFQ X-Received: by 10.28.224.86 with SMTP id x83mr23701650wmg.36.1449508914625; Mon, 07 Dec 2015 09:21:54 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <562FC0A8.1080603@openwall.com> References: <20151026200605.GI8645@brightrain.aerifal.cx> <562FC0A8.1080603@openwall.com> From: Paul Pluzhnikov Date: Mon, 7 Dec 2015 09:21:24 -0800 Message-ID: Subject: Re: [patch] Fix BZ 19165 -- overflow in fread / fwrite To: Alexander Cherepanov Cc: Rich Felker , GLIBC Devel , Florian Weimer , "Joseph S. Myers" Ok, let's try one more time. Previous attempt: https://sourceware.org/ml/libc-alpha/2015-10/msg00894.html Tested on Linux/x86_64 with 6.0.0 (r222386). Testing with (gcc-4.8 Ubuntu 4.8.4-2ubuntu1~14.04) 4:4.8.2-1ubuntu6 fails for apparently unrelated reasons: build/math/test-double-vlen2-wrappers.o: In function `cos_vlen2': /glibc-git/math/../sysdeps/x86_64/fpu/test-double-vlen2-wrappers.c:24: undefined reference to `_ZGVbN2v_cos' /glibc-git/build-system-gcc/math/test-double-vlen2-wrappers.o: In function `sin_vlen2': /glibc-git/math/../sysdeps/x86_64/fpu/test-double-vlen2-wrappers.c:25: undefined reference to `_ZGVbN2v_sin' ... etc. Thanks, 2015-12-07 Paul Pluzhnikov [BZ #19165] * misc/sys/cdefs.h (__umul_size_t_overflow): New. (__umul_size_t_saturated): New. * libio/iofread.c (_IO_fread): Protect against overflow. * libio/iofread_u.c (__fread_unlocked): Likewise. * b/libio/iofwrite.c (_IO_fwrite): Likewise. * libio/iofwrite_u.c (fwrite_unlocked): Likewise. diff --git a/libio/iofread.c b/libio/iofread.c index eb69b05..515f862 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 = __umul_size_t_saturated (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..9e2d213 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 = __umul_size_t_saturated (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..bb34257 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 = __umul_size_t_saturated (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..6e002bf 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 = __umul_size_t_saturated (size, count); _IO_size_t written = 0; CHECK_FILE (fp, 0); if (request == 0) diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h index 99e94cc..4474f09 100644 --- a/misc/sys/cdefs.h +++ b/misc/sys/cdefs.h @@ -441,4 +441,37 @@ # endif #endif +#include /* for size_t. */ + +__always_inline +static int +__umul_size_t_overflow (size_t a, size_t b) +{ +#if __GNUC_PREREQ (5, 0) + size_t result; + + /* _Static_assert, but without triggering conformance test failures. */ + __attribute__ ((__unused__)) + char sizeof_size_t_eq_sizeof_ulong[ + sizeof (size_t) == sizeof (unsigned long) ? 1 : -1]; + + return __builtin_umull_overflow (a, b, &result); +#else + const size_t half_size_t = (size_t) 1 << 4 * sizeof (size_t); + const size_t size_max = (size_t) -1; + + return __glibc_unlikely ((a | b) >= half_size_t) && b > 1 && a > size_max / b; +#endif +} + +__always_inline +static size_t +__umul_size_t_saturated (size_t a, size_t b) +{ + const size_t size_max = (size_t) -1; + + if (__umul_size_t_overflow (a, b)) return size_max; + return a * b; +} + #endif /* sys/cdefs.h */