Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752423AbcD2KLR (ORCPT ); Fri, 29 Apr 2016 06:11:17 -0400 Received: from mail-wm0-f51.google.com ([74.125.82.51]:35325 "EHLO mail-wm0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751215AbcD2KLP (ORCPT ); Fri, 29 Apr 2016 06:11:15 -0400 MIME-Version: 1.0 In-Reply-To: <20160428192009.lqoyl65l4lhnmecs@mguzik> References: <1461870258-17970-1-git-send-email-minipli@googlemail.com> <20160428192009.lqoyl65l4lhnmecs@mguzik> Date: Fri, 29 Apr 2016 13:11:14 +0300 Message-ID: Subject: Re: [PATCH] proc: prevent accessing /proc//environ until it's ready From: Alexey Dobriyan To: Mateusz Guzik Cc: Mathias Krause , Andrew Morton , Linux Kernel , Emese Revfy , Pax Team , Al Viro , Cyrill Gorcunov , Jarod Wilson Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1919 Lines: 45 On Thu, Apr 28, 2016 at 10:20 PM, Mateusz Guzik wrote: > On Thu, Apr 28, 2016 at 09:04:18PM +0200, Mathias Krause wrote: >> If /proc//environ gets read before the envp[] array is fully set >> up in create_{aout,elf,elf_fdpic,flat}_tables(), we might end up trying >> to read more bytes than are actually written, as env_start will already >> be set but env_end will still be zero, making the range calculation >> underflow, allowing to read beyond the end of what has been written. >> >> Fix this as it is done for /proc//cmdline by testing env_end for >> zero. It is, apparently, intentionally set last in create_*_tables(). >> >> This bug was found by the PaX size_overflow plugin that detected the >> arithmetic underflow of 'this_len = env_end - (env_start + src)' when >> env_end is still zero. >> >> Reported-at: https://forums.grsecurity.net/viewtopic.php?f=3&t=4363 >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=116461 >> --- >> fs/proc/base.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/fs/proc/base.c b/fs/proc/base.c >> index 4f764c2ac1a5..45f2162e55b2 100644 >> --- a/fs/proc/base.c >> +++ b/fs/proc/base.c >> @@ -955,7 +955,8 @@ static ssize_t environ_read(struct file *file, char __user *buf, >> struct mm_struct *mm = file->private_data; >> unsigned long env_start, env_end; >> >> - if (!mm) >> + /* Ensure the process spawned far enough to have an environment. */ >> + if (!mm || !mm->env_end) >> return 0; >> >> page = (char *)__get_free_page(GFP_TEMPORARY); > > In this case get_cmdline in mm/util.c should also be patched for > completness. It tests for arg_end, but later accesses env_end. Sort of. get_cmdline() is only really used in audit code applied to an exiting process which has cmdline setup long ago. Should have rewrote /proc/*/environ as well... :-( Alexey