From patchwork Thu Aug 27 21:20:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 511439 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 031E61400A0 for ; Fri, 28 Aug 2015 07:21:05 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=moDY1rLi; 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 :content-transfer-encoding; q=dns; s=default; b=f+FX781COnJhOhLb a5JLjxLUiSYWlYA4+US4cEHMfnvncjouA27+d3Ecl2O1PtLlrJHjU79nhkNkoGRk dj/eEdRBEeDfVJ3jwWWuWsbvahBDex0txKDMqNQL3spT4QYPKGVtIBM86BaIEEaE 4NBFf1RdqmahuNRgdGXrzimBUHI= 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 :content-transfer-encoding; s=default; bh=cjd+1uEO9GCfQcJYZb3v+c KCORY=; b=moDY1rLikZbhwWgzXnHAFP8q53PxM1m2GE3rfGq8PXbFbYWnm0Q/YG vzV6mtSCHR69oNLEnmypPu/SFjzb4ioTvTiF1KqVY8vZ1V1xMd3FCKySiGYCviQc 9YbT9+pDKLqmqfk67XOQGcuUADSfEZKsqMe5fHfXJl0GsoCKSKKOU= Received: (qmail 98375 invoked by alias); 27 Aug 2015 21:20:58 -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 98330 invoked by uid 89); 27 Aug 2015 21:20:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-yk0-f181.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=tQdfFxX6XZUys5Ssj49GkNNfnDAMJb5rVLQeygT3scc=; b=Jxf0PMcM0RifrixzNf2exv1XUIgbhsqByzooQAgM0Rx+5zvqVdgHRpHXRLcIDDvsG5 Gp3QpdyS5wjX5iFud+ptRrTBuDv7ml+z9TbpinnMSUqiH2zgLMRTG/q+0r7E8lhx3Ims BpFr8iJtqEcVIQnpiqMEIFdtqfHqlvmRzuCq0B4afOde+DfenejafD7GZzXE9vsXgEHX RLUZSf/rMq+Qyj381HXt6jlhGqR05TdXXjm9eIyySm2OhYHlj63oH6euC3bGz1DnNbXV ctdKZehhrpHfYYPm+zoSu3dmN739PpeUAedWx9tEASqtyY2QVED1LgO5mWlrr8lzcxjG hy+Q== X-Gm-Message-State: ALoCoQmGXKTB3k5WxKsJUqAMXzYcjiychuAF/u9Hykp5/lrnfT6iJeFbn0aZeLBBYeg35fWirhsx X-Received: by 10.129.40.207 with SMTP id o198mr3900104ywo.145.1440710453811; Thu, 27 Aug 2015 14:20:53 -0700 (PDT) Subject: Re: [PATCH] Fix wordsize-32 mmap offset for negative value (BZ#18877) To: libc-alpha@sourceware.org References: <55DF6F59.3000404@linaro.org> <20150827205128.GA2635@altlinux.org> From: Adhemerval Zanella Message-ID: <55DF7F32.2010308@linaro.org> Date: Thu, 27 Aug 2015 18:20:50 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150827205128.GA2635@altlinux.org> On 27-08-2015 17:51, Dmitry V. Levin wrote: > On Thu, Aug 27, 2015 at 05:13:13PM -0300, Adhemerval Zanella wrote: >> This patch fixes the default wordsize-32 mmap implementation offset >> calculation for negative values. Current code uses signed shift >> operation to calculate the multiple size to use with syscall and >> it is implementation defined. Change it to use a division base >> on mmap page size (default being as before, 4096). >> >> Tested on armv7hf. >> >> [BZ #18877] >> * misc/Makefile (tests): Add tst-mmap >> * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix >> offset calculation for negative values. >> * misc/tst-mmap.c: New file. > > As we have posix/tst-mmap.c already, my idea was to call it > tst-mmap-offset.c, > >> + char fname[] = "tst-mmap-offset-XXXXXX"; > > hence the name of this temporary file. > >> - offset >> MMAP_PAGE_SHIFT); >> + offset/MMAP_PAGE_UNIT); > > offset / MMAP_PAGE_UNIT Thanks for the review, what about now: --- [BZ #18877] * posix/Makefile (tests): Add tst-mmap * sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c (__mmap): Fix offset calculation for negative values. * posix/tst-mmap-offset.c: New file. -- diff --git a/posix/Makefile b/posix/Makefile index 15e8818..39423a9 100644 --- a/posix/Makefile +++ b/posix/Makefile @@ -64,7 +64,7 @@ routines := \ aux := init-posix environ tests := tstgetopt testfnm runtests runptests \ tst-preadwrite tst-preadwrite64 test-vfork regexbug1 \ - tst-mmap tst-getaddrinfo tst-truncate \ + tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \ tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \ tst-chmod bug-regex1 bug-regex2 bug-regex3 bug-regex4 \ tst-gnuglob tst-regex bug-regex5 bug-regex6 bug-regex7 \ diff --git a/posix/tst-mmap-offset.c b/posix/tst-mmap-offset.c new file mode 100644 index 0000000..15dc4da --- /dev/null +++ b/posix/tst-mmap-offset.c @@ -0,0 +1,67 @@ +/* BZ #18877 mmap offset test. + + Copyright (C) 2015 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include + +static int +printmsg (int rc, const char *msg) +{ + printf ("%s failed: %m\n", msg); + return rc; +} + +/* Check if negative offset are handled correctly by mmap. */; +static int +do_test (void) +{ + const int prot = PROT_READ | PROT_WRITE; + const int flags = MAP_SHARED; + const unsigned long length = 0x10000; + const unsigned long offset = 0xace00000; + const unsigned long size = offset + length; + void *addr; + int fd; + char fname[] = "tst-mmap-offset-XXXXXX"; + + fd = mkstemp64 (fname); + if (fd < 0) + return printmsg (1, "mkstemp"); + + if (unlink (fname)) + return printmsg (1, "unlink"); + + if (ftruncate64 (fd, size)) + return printmsg (0, "ftruncate64"); + + addr = mmap (NULL, length, prot, flags, fd, offset); + if (MAP_FAILED == addr) + return printmsg (1, "mmap"); + + /* This memcpy is likely to SIGBUS if mmap has messed up with offset. */ + memcpy (addr, fname, sizeof (fname)); + + return 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c b/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c index 24835ce..82d8920 100644 --- a/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c +++ b/sysdeps/unix/sysv/linux/generic/wordsize-32/mmap.c @@ -21,20 +21,20 @@ #include #include -#ifndef MMAP_PAGE_SHIFT -#define MMAP_PAGE_SHIFT 12 +#ifndef MMAP_PAGE_UNIT +#define MMAP_PAGE_UNIT 4096UL #endif __ptr_t __mmap (__ptr_t addr, size_t len, int prot, int flags, int fd, off_t offset) { - if (offset & ((1 << MMAP_PAGE_SHIFT) - 1)) + if (offset & (MMAP_PAGE_UNIT - 1)) { __set_errno (EINVAL); return MAP_FAILED; } return (__ptr_t) INLINE_SYSCALL (mmap2, 6, addr, len, prot, flags, fd, - offset >> MMAP_PAGE_SHIFT); + offset / MMAP_PAGE_UNIT); } weak_alias (__mmap, mmap)