From: Ryan Mallon <rmallon@gmail.com>
To: halfdog <me@halfdog.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Mark Salter <msalter@redhat.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Pass correct length to strnlen_user in fs/exec.c
Date: Sat, 17 Sep 2011 10:03:21 +1000 [thread overview]
Message-ID: <4E73E3C9.4040702@gmail.com> (raw)
In-Reply-To: <4E737B16.2090104@halfdog.net>
On 17/09/11 02:36, halfdog wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Ryan Mallon wrote:
>
>>> ...
>> That's fine. I originally went looking after a discussion with
>> Mark about the weird strnlen_user semantics and this usage looked
>> incorrect to me because it wasn't obviously checking >=
>> MAX_ARG_STRLEN.
>>
>> The rework I think makes it a bit more clear and passes the correct
>> max length to strnlen_user. Its a bit odd to pass MAX_ARG_STRLEN
>> and then check if it is longer than bprm->len, and I guess assumes
>> that bprm->len is less than MAX_ARG_STRLEN.
>>
>> Feel free to drop the patch if you think it is just churn.
> As you are already busy with fs/exec.c and strnlen_user, could you
> please check, if the timerace (argv/envp corruption) using
> strnlen_user could also be fixed within the same patch (if bug still
> in kernel). See
>
> https://bugzilla.kernel.org/show_bug.cgi?id=39222
With kernel.org currently being down I cannot see that bug report
unfortunately, so I don't know what the bug you are referring to is.
> The race occurs when one thread issues exec while another one with
> shared memory pages is flipping char/0-bytes of the args or envptrs,
> causing random split (number greater maxargs - no problem, silently
> droped)/fusion (greater MAX_ARG_STRLEN - might harm dumb programs) or
> removal of all args including program name on top of new stack when
> calling via binfmt handlers (might have influenced old ldlinux/libc
> pre 2009?? when handling suid binaries)
It doesn't sound like my patch would have fixed that. The only issue
that my patch would have corrected is the case where bprm->len is
greater than MAX_ARG_STRLEN. MAX_ARG_STRLEN is pretty big though (32 *
PAGE_SIZE), so I don't see that being an issue very often.
~Ryan
prev parent reply other threads:[~2011-09-17 0:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-08 0:39 [PATCH] Pass correct length to strnlen_user in fs/exec.c Ryan Mallon
2011-09-08 0:39 ` Ryan Mallon
2011-09-08 23:52 ` Andrew Morton
2011-09-09 0:02 ` Ryan Mallon
2011-09-16 16:36 ` halfdog
2011-09-17 0:03 ` Ryan Mallon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E73E3C9.4040702@gmail.com \
--to=rmallon@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=me@halfdog.net \
--cc=msalter@redhat.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.