From patchwork Thu Nov 23 16:38:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 24476 Received: (qmail 76546 invoked by alias); 23 Nov 2017 16:39:01 -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 76536 invoked by uid 89); 23 Nov 2017 16:39:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=certify X-HELO: mail-qt0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=FeWm57JBbm6Z3BHZaRmZsDS2JVTSw9zBTwwsO/HaBo4=; b=A2xKWWHCEtcnVD8s3litsNcsqzFsojH0HCmJ1ssf+WwnZ4hB22E2p6g4WDZEFfC9BV AOVR1fR2EaQgD3xcLzKYjwqyXFo1KAp6Sh0/0TQ1GZcMGEBG95gxmZnJxyVEXlSapcpE 1kDLVFJyMZEpgtQI+Nl/10hVzBRloQC+GQJAyjsnMGyvlmkDOk+Wz0sRp64LV965VDFp e5SIMkyD55QtMZ1uFeP5hd0ux5Q4x2Rzpw8MAY6LMvGIJ/Erz43WZB0uNgLa84wrlJCc ooDUyzkmtYxDeZaKGFGOPyLN+GbDp7LAHyCFqp6yRO3S3jjTmN7WQXcM/ag5iTfEtQrZ gekw== X-Gm-Message-State: AJaThX7suYADRZSAebINCSK7clV3n7E710+LR5Q1xv/NmDC4rgTcjMoc +VR3OLzzVWYVma8fwOZ8IXfKeCzxCyE= X-Google-Smtp-Source: AGs4zMbbL91JEZDxj2L7rbACKulyZBgaSIKB5Jr/H9N85N8gdNbzVO/OZ85BvN6wpVgRtxjzgG9gJg== X-Received: by 10.200.43.119 with SMTP id 52mr19232322qtv.143.1511455137544; Thu, 23 Nov 2017 08:38:57 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH] posix: Fix generic p{read, write}v buffer allocation (BZ#22457) Date: Thu, 23 Nov 2017 14:38:50 -0200 Message-Id: <1511455130-19179-1-git-send-email-adhemerval.zanella@linaro.org> As described in BZ#22457 an interpose malloc can free an invalid pointer for fallback preadv implementation. Fortunately this is just and issue on microblaze-linux-gnu running kernels older than 3.15. This patch fixes it by calling mmap/unmap instead of posix_memalign/ free. A sanity check is address to certify buffer alignment (since POSIX does not enforces a particular alignment on returned mmap value, just for MMAP_FIXED input). Checked on microblaze-linux-gnu check with run-built-tests=no and by using the sysdeps/posix implementation on x86_64-linux-gnu (just for sanity test where it shown no regression). * include/libc-pointer-arith.h (PTR_IS_ALIGNED): New macro. * sysdeps/posix/preadv_common.c (PREADV): Use mmap/munmap instead of posix_memalign/free. * sysdeps/posix/pwritev_common.c (PWRITEV): Likewise. --- ChangeLog | 7 +++++++ include/libc-pointer-arith.h | 4 ++++ sysdeps/posix/preadv_common.c | 10 +++++++--- sysdeps/posix/pwritev_common.c | 10 +++++++--- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/include/libc-pointer-arith.h b/include/libc-pointer-arith.h index 715cbc1..c918023 100644 --- a/include/libc-pointer-arith.h +++ b/include/libc-pointer-arith.h @@ -57,4 +57,8 @@ #define PTR_ALIGN_UP(base, size) \ ((__typeof__ (base)) ALIGN_UP ((uintptr_t) (base), (size))) +/* Check if base is aligned to boundary. */ +#define PTR_IS_ALIGNED(base, boundary) \ + (((uintptr_t) (base) & ((uintptr_t) ((boundary)) - 1)) == 0) + #endif diff --git a/sysdeps/posix/preadv_common.c b/sysdeps/posix/preadv_common.c index 37efdc0..099c37b 100644 --- a/sysdeps/posix/preadv_common.c +++ b/sysdeps/posix/preadv_common.c @@ -24,6 +24,7 @@ #include #include +#include /* Read data from file descriptor FD at the given position OFFSET without change the file pointer, and put the result in the buffers @@ -54,8 +55,11 @@ PREADV (int fd, const struct iovec *vector, int count, OFF_T offset) but 1. it is system specific (not meant in generic implementation), and 2. it would make the implementation more complex, and 3. it will require another syscall (fcntl). */ - void *buffer = NULL; - if (__posix_memalign (&buffer, GLRO(dl_pagesize), bytes) != 0) + size_t mmap_size = ALIGN_UP (bytes, GLRO(dl_pagesize)); + void *buffer = __mmap (NULL, mmap_size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (__glibc_unlikely (buffer == MAP_FAILED) + || __glibc_unlikely (!PTR_IS_ALIGNED (buffer, GLRO(dl_pagesize)))) return -1; ssize_t bytes_read = PREAD (fd, buffer, bytes, offset); @@ -78,6 +82,6 @@ PREADV (int fd, const struct iovec *vector, int count, OFF_T offset) } end: - free (buffer); + __munmap (buffer, mmap_size); return bytes_read; } diff --git a/sysdeps/posix/pwritev_common.c b/sysdeps/posix/pwritev_common.c index 0383065..610c54e 100644 --- a/sysdeps/posix/pwritev_common.c +++ b/sysdeps/posix/pwritev_common.c @@ -24,6 +24,7 @@ #include #include +#include /* Write data pointed by the buffers described by IOVEC, which is a vector of COUNT 'struct iovec's, to file descriptor FD at the given @@ -54,8 +55,11 @@ PWRITEV (int fd, const struct iovec *vector, int count, OFF_T offset) but 1. it is system specific (not meant in generic implementation), and 2. it would make the implementation more complex, and 3. it will require another syscall (fcntl). */ - void *buffer = NULL; - if (__posix_memalign (&buffer, GLRO(dl_pagesize), bytes) != 0) + size_t mmap_size = ALIGN_UP (bytes, GLRO(dl_pagesize)); + void *buffer = __mmap (NULL, mmap_size, PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (__glibc_unlikely (buffer == MAP_FAILED) + || __glibc_unlikely (!PTR_IS_ALIGNED (buffer, GLRO(dl_pagesize)))) return -1; /* Copy the data from BUFFER into the memory specified by VECTOR. */ @@ -66,7 +70,7 @@ PWRITEV (int fd, const struct iovec *vector, int count, OFF_T offset) ssize_t ret = PWRITE (fd, buffer, bytes, offset); - free (buffer); + __munmap (buffer, mmap_size); return ret; }