Re: [PATCH (repost)] umh: fix refcount underflow in fork_usermode_blob().

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH (repost)] umh: fix refcount underflow in fork_usermode_blob().
Date: Sun, 29 Mar 2020 13:28:41 +0900	[thread overview]
Message-ID: <f8099e95-c0fc-d133-471e-e0ba97d2230e@i-love.sakura.ne.jp> (raw)
In-Reply-To: <20200328175512.e89ff65333e6c65fea211c12@linux-foundation.org>

On 2020/03/29 9:55, Andrew Morton wrote:
> On Fri, 27 Mar 2020 09:51:34 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
> 
>> Since free_bprm(bprm) always calls allow_write_access(bprm->file) and
>> fput(bprm->file) if bprm->file is set to non-NULL, __do_execve_file()
>> must call deny_write_access(file) and get_file(file) if called from
>> do_execve_file() path. Otherwise, use-after-free access can happen at
>> fput(file) in fork_usermode_blob().
>>
>>   general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] SMP DEBUG_PAGEALLOC
>>   CPU: 3 PID: 4131 Comm: insmod Tainted: G           O      5.6.0-rc5+ #978
>>   Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/29/2019
>>   RIP: 0010:fork_usermode_blob+0xaa/0x190
> 
> This is rather old code - what casued this to be observed now?  Some
> unusual userspace behaviour?

I'm attempting to fix a regression for TOMOYO caused by commit 51f39a1f0cea1cac
("syscalls: implement execveat() system call") in 3.19 that execve() request fails
if fd argument is not AT_FDCWD, for TOMOYO needs to re-calculate "requested pathname
using AT_SYMLINK_NOFOLLOW" from LSM hook. That regression was practically not a big
problem because 99%+ of execve() request used AT_FDCWD, for in general executing a
program involves opening external libraries which have to be accessible from mount
namespace where execve() was requested. But commit 449325b52b7a6208 ("umh: introduce
fork_usermode_blob() helper") in 4.18 was a rather unique change that allows file-less
execution of a program, which means that file-less execution request fails because
TOMOYO can never calculate "requested pathname using AT_SYMLINK_NOFOLLOW".

This patch itself does not fix the regression for TOMOYO. But this patch fixes a
memory corruption bug which should be applied regardless of the regression for TOMOYO.
Although Al does not like this patch, I'd like to keep this patch minimal so that
RedHat folks can easily backport this patch to RHEL 8 (which uses 4.18).