From patchwork Thu Jul 16 08:00:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddhesh Poyarekar X-Patchwork-Id: 7715 Received: (qmail 63215 invoked by alias); 16 Jul 2015 08:00:46 -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 63203 invoked by uid 89); 16 Jul 2015 08:00:45 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.1 required=5.0 tests=AWL, BAYES_05, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com From: "Siddhesh Poyarekar" To: libc-alpha@sourceware.org Cc: roland@hack.frob.com Subject: [PATCH] Avoid mapping past end of shared object (BZ #18685) Date: Thu, 16 Jul 2015 13:30:25 +0530 Message-Id: <1437033625-13561-1-git-send-email-siddhesh@redhat.com> Some valid ELF objects, like .debug files may refer to sections outside themselves since they're loaded and patched up to their parent ELF. Calling ldd on them may result in a segfault since it may try to read beyond the end of the mapping. Red Hat bz: https://bugzilla.redhat.com/show_bug.cgi?id=741105 has an example, although I haven't been able to find a sample reproducer file immediately. This patch has been carried in Fedora and RHEL for a couple of years now. Also tested on x86_64 to verify that there are no regressions. [BZ #18685] * sysdeps/generic/dl-fileid.h (_dl_get_file_id): Accept pointer to a struct stat64. * sysdeps/posix/dl-fileid.h (_dl_get_file_id): Likewise. * elf/dl-load.c (_dl_map_object_from_fd): Avoid mapping past end of ELF file. --- elf/dl-load.c | 14 +++++++++++++- sysdeps/generic/dl-fileid.h | 3 ++- sysdeps/posix/dl-fileid.h | 12 +++++------- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/elf/dl-load.c b/elf/dl-load.c index 0c052e4..f8aaa60 100644 --- a/elf/dl-load.c +++ b/elf/dl-load.c @@ -880,7 +880,8 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp, /* Get file information. */ struct r_file_id id; - if (__glibc_unlikely (!_dl_get_file_id (fd, &id))) + struct stat64 st; + if (__glibc_unlikely (!_dl_get_file_id (fd, &id, &st))) { errstring = N_("cannot stat shared object"); call_lose_errno: @@ -1076,6 +1077,17 @@ _dl_map_object_from_fd (const char *name, int fd, struct filebuf *fbp, goto call_lose; } + if (__glibc_unlikely (ph->p_offset + ph->p_filesz > st.st_size)) + { + /* If the segment requires zeroing of part of its last + page, we'll crash when accessing the unmapped page. + There's still a possibility of a race, if the shared + object is truncated between the fxstat above and the + memset below. */ + errstring = N_("ELF load command past end of file"); + goto call_lose; + } + struct loadcmd *c = &loadcmds[nloadcmds++]; c->mapstart = ph->p_vaddr & ~(GLRO(dl_pagesize) - 1); c->mapend = ((ph->p_vaddr + ph->p_filesz + GLRO(dl_pagesize) - 1) diff --git a/sysdeps/generic/dl-fileid.h b/sysdeps/generic/dl-fileid.h index 2cbd21d..9b7f410 100644 --- a/sysdeps/generic/dl-fileid.h +++ b/sysdeps/generic/dl-fileid.h @@ -29,7 +29,8 @@ struct r_file_id On error, returns false, with errno set. */ static inline bool _dl_get_file_id (int fd __attribute__ ((unused)), - struct r_file_id *id __attribute__ ((unused))) + struct r_file_id *id __attribute__ ((unused)), + struct stat64_t *st __attribute__((unused))) { return true; } diff --git a/sysdeps/posix/dl-fileid.h b/sysdeps/posix/dl-fileid.h index d0d5436..7115c3b 100644 --- a/sysdeps/posix/dl-fileid.h +++ b/sysdeps/posix/dl-fileid.h @@ -27,18 +27,16 @@ struct r_file_id ino64_t ino; }; -/* Sample FD to fill in *ID. Returns true on success. +/* Sample FD to fill in *ID and *ST. Returns true on success. On error, returns false, with errno set. */ static inline bool -_dl_get_file_id (int fd, struct r_file_id *id) +_dl_get_file_id (int fd, struct r_file_id *id, struct stat64 *st) { - struct stat64 st; - - if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, &st) < 0)) + if (__glibc_unlikely (__fxstat64 (_STAT_VER, fd, st) < 0)) return false; - id->dev = st.st_dev; - id->ino = st.st_ino; + id->dev = st->st_dev; + id->ino = st->st_ino; return true; }