[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jJXFP-1rEeYtMfqrSUz2oz8TWcjr89zL6-MPsNS7rwywg@mail.gmail.com>
Date: Thu, 14 Feb 2019 22:27:07 -0800
From: Kees Cook <keescook@...omium.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Oleg Nesterov <oleg@...hat.com>,
Samuel Dionne-Riel <samuel@...nne-riel.com>,
Richard Weinberger <richard.weinberger@...il.com>,
Graham Christensen <graham@...hamc.com>,
Michal Hocko <mhocko@...e.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] exec: load_script: Do not exec truncated interpreter path
On Thu, Feb 14, 2019 at 10:14 PM Kees Cook <keescook@...omium.org> wrote:
> But, as it turns out, the above is actually wrong too (yay for my test
> cases): the NUL termination before the loop (line 45) may blow away
> the newline at byte 127. So we need to actually manually scan for the
> newline while doing the out-of-bounds checking. (This was part of
> Oleg's original "don't blindly truncate" rationale in the reverted
> patch.)
Actually, though, this passes my regression tests:
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 7cde3f46ad26..6d7787e35d76 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -42,9 +42,18 @@ static int load_script(struct linux_binprm *bprm)
fput(bprm->file);
bprm->file = NULL;
- bprm->buf[BINPRM_BUF_SIZE - 1] = '\0';
- if ((cp = strchr(bprm->buf, '\n')) == NULL)
- cp = bprm->buf+BINPRM_BUF_SIZE-1;
+ if ((cp = strnchr(bprm->buf, BINPRM_BUF_SIZE, '\n')) == NULL) {
+ bool truncated = true;
+
+ for (cp = bprm->buf+2; cp < bprm->buf+BINPRM_BUF_SIZE-1 &&
+ ((*cp == ' ') || (*cp == '\t')); cp++);
+ for (; cp < bprm->buf+BINPRM_BUF_SIZE-1; cp++) {
+ if ((*cp == ' ') || (*cp == '\t'))
+ truncated = false;
+ }
+ if (truncated)
+ return -ENOEXEC; /* Interpreter truncated */
+ }
*cp = '\0';
while (cp > bprm->buf) {
cp--;
I still want to add all the comments, though. :)
(The above still seems uglier to me than just collecting the
information as we need it, but I'll do whatever.)
What do you think?
--
Kees Cook
Powered by blists - more mailing lists