Re: Userspace regression in LTS and stable kernels - Samuel Dionne-Riel

From: Samuel Dionne-Riel <samuel@dionne-riel.com>
To: Kees Cook <keescook@chromium.org>
Cc: Richard Weinberger <richard.weinberger@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Graham Christensen <graham@grahamc.com>,
	Oleg Nesterov <oleg@redhat.com>, Michal Hocko <mhocko@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: Userspace regression in LTS and stable kernels
Date: Wed, 13 Feb 2019 22:16:13 -0500	[thread overview]
Message-ID: <CAN1fySWW7y=nt72kHKydG-KrcrZhsj0YKZExyRc_OmAb_+QqAg@mail.gmail.com> (raw)
In-Reply-To: <CAGXu5jKXUhYB9xBLMWTvJiJEVPyRd=RsZTOCSvbNv7i6pEhtmg@mail.gmail.com>

On 13/02/2019, Kees Cook <keescook@chromium.org> wrote:
> The original problem that was trying to be fixed here was to disallow
> execution of a truncated interpreter path. It was assumed argument
> truncate was just as bad, but it's not, since the interpreter can (and
> does!) re-read the script to get the right arguments.
>
> So, I've sent a fix-up patch that should disallow the path truncation,
> but pass through the argument truncation as before. This passes all
> the tests I built:
>
> [...]
>
> Are you able to test the patch and report back?

The patch works as implemented. It also fixes the specific regression
which affected NixOS. This was verified to work once applied to 4.14
in our testing infra. Confidence is high enough that I don't think I
need to test other LTS/stable versions.

Though, I have one minor doubt in mind. Looking at man 2 execve,

ENOEXEC
   An executable is not in a recognized format, is for the wrong
   architecture, or has some other format error that means it cannot
be executed.

I can see "or some other format error" could be misapplied to mean
ENOEXEC on failure to read the shebang, but I'm thinking it's kinda
abusing the meaning behind the failure. The format was recognized, as
a shebang, but it was impossible to use the shebang. If I were to
misuse an error code, I would probably misuse ENAMETOOLONG. I'm still
doubting ENOEXEC is safe to not cause issues since a truncated
interpreter name (not shebang) will end up with a different behaviour
than expected in the exec(3) userspace scenario, where with ENOEXEC
the shell will be used instead of failing.

Though, this is a different face to the same root regression reported
here; our initial issue with the regression can be deemed fully
resolved with the patch.

In all cases, I think the man page will need an update to describe the
new failure mode with truncated shebangs, and describe the non-failure
mode when truncating arguments.

> Thanks again for bringing this to our attention!

Thanks for the quick turnaround!

-- 
— Samuel Dionne-Riel