From patchwork Wed May 11 10:37:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hector Marco-Gisbert X-Patchwork-Id: 9067881 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 9C6EFBF29F for ; Wed, 11 May 2016 10:55:55 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AE70A2012D for ; Wed, 11 May 2016 10:55:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 29AB720155 for ; Wed, 11 May 2016 10:55:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751450AbcEKKzv (ORCPT ); Wed, 11 May 2016 06:55:51 -0400 Received: from smtpsalv.cc.upv.es ([158.42.249.11]:58165 "EHLO smtpsalv.upv.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751338AbcEKKzv (ORCPT ); Wed, 11 May 2016 06:55:51 -0400 X-Greylist: delayed 1098 seconds by postgrey-1.27 at vger.kernel.org; Wed, 11 May 2016 06:55:50 EDT Received: from smtpx.upv.es (smtpxv.cc.upv.es [158.42.249.46]) by smtpsalv.upv.es (8.14.4/8.14.4) with ESMTP id u4BAbS7Q003118 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 11 May 2016 12:37:29 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=upv.es; s=default; t=1462963049; bh=IZ3/0G7kpemEwuVKvwJyFtDp6gcchDXfPZlIlRWZqBU=; h=From:To:Cc:Subject:Date; b=lNMEKBgGHGF9fQ9LYtRJKm/x0HEBDegUZMhN7yqfnsk/J6CJiHUA7HKYWlTI3UxHD /3j9xH4PHKkvNQS4X7fhtz2M0fOYJloQwkSzjnAMXwVfuoZ8duTF7NEofQ/lxJINVO /DqYtHhuG4R89LXXfdJyTIAFpAxXMkz9LbCaCdwFllLP5VDTk5FaeS2zTKncob9tWz ggIZXtYohb/6j4qMUhi5cNCd66XeiIpG1vHktIhXJVta8TJBbCHQVSmZXIFnzlc8W/ SxxBPIYcgNRSw7fvuNHAtTlw3MHiIjtjTSORhterqtc6ESCFn86PPD8tR5V9hs+kjk O2nPicPNkChYA== Received: from smtp.upv.es (smtpv.cc.upv.es [158.42.249.16]) by smtpx.upv.es (8.14.3/8.14.3) with ESMTP id u4BAbSHq012165; Wed, 11 May 2016 12:37:28 +0200 Received: from localhost.localdomain (trinca.disca.upv.es [158.42.52.215]) (authenticated bits=0) by smtp.upv.es (8.14.4/8.14.4) with ESMTP id u4BAbPfq007044 (version=TLSv1/SSLv3 cipher=AES128-SHA256 bits=128 verify=NO); Wed, 11 May 2016 12:37:28 +0200 From: Hector Marco-Gisbert To: linux-kernel@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, Alexander Viro , kees Cook , Ismael Ripoll Ripoll , Hector Marco-Gisbert Subject: [PATCH] Fix bss mapping for the interpreter in binfmt_elf Date: Wed, 11 May 2016 12:37:00 +0200 Message-Id: <1462963020-11097-1-git-send-email-hecmargi@upv.es> X-Mailer: git-send-email 1.9.1 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-8.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP While working on a new ASLR for userspace we detected an error in the interpret loader. The size of the bss section for some interpreters is not correctly calculated resulting in unnecessary calls to vm_brk() with enormous size values. The bug appears when loading some interpreters with a small bss size. Once the last loadable segment has been loaded, the bss section is zeroed up to the page boundary and the elf_bss variable is updated to this new page boundary. Because of this update (alignment), the last_bss could be less than elf_bss and the subtraction "last_bss - elf_bss" value could overflow. Although it is quite easy to check this error, it has not been manifested because some peculiarities of the bug. Following is a brief description: $ size /lib32/ld-2.19.so text data bss dec hex filename 128456 2964 192 131612 2021c /lib32/ld-2.19.so An execution example with: - last_bss: 0xb7794938 - elf_bss: 0xb7794878 From fs/binfmt_elf.c: --------------------------------------------------------------------------- if (last_bss > elf_bss) { /* * Now fill out the bss section. First pad the last page up * to the page boundary, and then perform a mmap to make sure * that there are zero-mapped pages up to and including the * last bss page. */ if (padzero(elf_bss)) { error = -EFAULT; goto out; } /* What we have mapped so far */ elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1); <---------- elf_bss here is 0xb7795000 /* Map the last of the bss segment */ error = vm_brk(elf_bss, last_bss - elf_bss); <-- overflow! if (BAD_ADDR(error)) goto out; } error = load_addr; out: return error; } --------------------------------------------------------------------------- The size value requested to the vm_brk() call (last_bss - elf_bss) is 0xfffffffffffff938 and internally this size is page aligned in the do_brk() function resulting in a 0 length request. static unsigned long do_brk(unsigned long addr, unsigned long len) { ... len = PAGE_ALIGN(len); if (!len) return addr; Since a segment of 0 bytes is perfectly valid, it returns the requested address to vm_brk() and because it is page aligned (done by the previous line to the vm_brk() call the "error" is not detected by "BAD_ADDR(error)" and the "load_elf_interp()" functions does not returns any error. Note that vm_brk() is not necessary at all. In brief, if the end of the bss is in the same page than the last segment loaded then the size of the last of bss segment is incorrectly calculated. This patch set up to the page boundary of the last_bss variable and only do the vm_brk() call when necessary. Signed-off-by: Hector Marco-Gisbert Acked-by: Ismael Ripoll Ripoll --- fs/binfmt_elf.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 81381cc..acfbdc2 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -526,7 +526,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, int load_addr_set = 0; unsigned long last_bss = 0, elf_bss = 0; unsigned long error = ~0UL; - unsigned long total_size; + unsigned long total_size, size; int i; /* First of all, some simple consistency checks */ @@ -626,11 +626,15 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, /* What we have mapped so far */ elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1); + last_bss = ELF_PAGESTART(last_bss + ELF_MIN_ALIGN - 1); /* Map the last of the bss segment */ - error = vm_brk(elf_bss, last_bss - elf_bss); - if (BAD_ADDR(error)) - goto out; + size = last_bss - elf_bss; + if (size) { + error = vm_brk(elf_bss, size); + if (BAD_ADDR(error)) + goto out; + } } error = load_addr;