From patchwork Thu Jun 22 17:38:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 9805007 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3BBE160386 for ; Thu, 22 Jun 2017 17:39:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3DCBB286FB for ; Thu, 22 Jun 2017 17:38:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3290D28706; Thu, 22 Jun 2017 17:38:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EEC7D286FC for ; Thu, 22 Jun 2017 17:38:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751677AbdFVRim (ORCPT ); Thu, 22 Jun 2017 13:38:42 -0400 Received: from mail-pg0-f42.google.com ([74.125.83.42]:32997 "EHLO mail-pg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751156AbdFVRil (ORCPT ); Thu, 22 Jun 2017 13:38:41 -0400 Received: by mail-pg0-f42.google.com with SMTP id f127so10672934pgc.0 for ; Thu, 22 Jun 2017 10:38:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition; bh=0FVYDIuHXVgUN53dIjPPToBhQC7Jyw/gtn9FZ8UzwKg=; b=FvtVwE6S+Toa9O7QcdYsL7nWYugdUZMK7daI9Wj32zEsznuof8dDs9ngrN2TFDsig1 M8wstZjqIX722CRn0emZOCG1odu1XWmqKqnC0HPZq0bx/KVoCBui+ni03HOYw1nWOcDu mjZHx24O3ms0EqF8CSwcG27n703DU2bnhxSrg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition; bh=0FVYDIuHXVgUN53dIjPPToBhQC7Jyw/gtn9FZ8UzwKg=; b=JitZWgK1dX4Q7Hq05eYZ6YuMiDXn054okqLS3IzN4LC++tk0SJRzgLDyrr7hciaL5K U+KwdcVxPUM8DLlAY7Ph3SDRjLJhaCvPi6dZq7WamYF8fxtlsVX3QWaQw+U2LGlAMdwH VnmK7onBxInV0J0ARhBKGvoQlUQtcYVh3agRmkCbfBySKPArDqgCNjgcf4hbg/Vlv7l9 ljX2UZE+sYsEBdd8W2XhPAyKs220dDSBnolZ9mhw9dfYeIxsnIdBUF+0ARPeFvZhWaAb ztKcgWVraZtuK5SS+w2kp/tiO2yE/WkWkGbpqTg+vgHy1sPOXHZ5TDY8Sh21EqaRgx8R 48sA== X-Gm-Message-State: AKS2vOyZRgJwajil4VrI3sziFS6n0FuX6iZHCebOmCK74N+QwYGZ4G37 Q1ym3t6ZZC8yFLBT X-Received: by 10.98.106.66 with SMTP id f63mr3843726pfc.169.1498153120313; Thu, 22 Jun 2017 10:38:40 -0700 (PDT) Received: from www.outflux.net (173-164-112-133-Oregon.hfc.comcastbusiness.net. [173.164.112.133]) by smtp.gmail.com with ESMTPSA id c6sm328481pfc.128.2017.06.22.10.38.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 22 Jun 2017 10:38:38 -0700 (PDT) Date: Thu, 22 Jun 2017 10:38:38 -0700 From: Kees Cook To: Andrew Morton Cc: Rik van Riel , Daniel Micay , Qualys Security Advisory , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Alexander Viro , Dmitry Safonov , Andy Lutomirski , Grzegorz Andrejczuk , Masahiro Yamada , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH] binfmt_elf: Safely increment argv pointers Message-ID: <20170622173838.GA43308@beast> MIME-Version: 1.0 Content-Disposition: inline Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When building the argv/envp pointers, the envp is needlessly pre-incremented instead of just continuing after the argv pointers are finished. In some (likely impossible) race where the strings could be changed from userspace between copy_strings() and here, it might be possible to confuse the envp position. Instead, just use sp like everything else. Signed-off-by: Kees Cook --- fs/binfmt_elf.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 7465c3ea5dd5..879ff9c7ffd0 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -163,8 +163,6 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, unsigned long p = bprm->p; int argc = bprm->argc; int envc = bprm->envc; - elf_addr_t __user *argv; - elf_addr_t __user *envp; elf_addr_t __user *sp; elf_addr_t __user *u_platform; elf_addr_t __user *u_base_platform; @@ -304,38 +302,38 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, /* Now, let's put argc (and argv, envp if appropriate) on the stack */ if (__put_user(argc, sp++)) return -EFAULT; - argv = sp; - envp = argv + argc + 1; - /* Populate argv and envp */ + /* Populate list of argv pointers back to argv strings. */ p = current->mm->arg_end = current->mm->arg_start; while (argc-- > 0) { size_t len; - if (__put_user((elf_addr_t)p, argv++)) + if (__put_user((elf_addr_t)p, sp++)) return -EFAULT; len = strnlen_user((void __user *)p, MAX_ARG_STRLEN); if (!len || len > MAX_ARG_STRLEN) return -EINVAL; p += len; } - if (__put_user(0, argv)) + if (__put_user(0, sp++)) return -EFAULT; - current->mm->arg_end = current->mm->env_start = p; + current->mm->arg_end = p; + + /* Populate list of envp pointers back to envp strings. */ + current->mm->env_end = current->mm->env_start = p; while (envc-- > 0) { size_t len; - if (__put_user((elf_addr_t)p, envp++)) + if (__put_user((elf_addr_t)p, sp++)) return -EFAULT; len = strnlen_user((void __user *)p, MAX_ARG_STRLEN); if (!len || len > MAX_ARG_STRLEN) return -EINVAL; p += len; } - if (__put_user(0, envp)) + if (__put_user(0, sp++)) return -EFAULT; current->mm->env_end = p; /* Put the elf_info on the stack in the right place. */ - sp = (elf_addr_t __user *)envp + 1; if (copy_to_user(sp, elf_info, ei_index * sizeof(elf_addr_t))) return -EFAULT; return 0;