From: Andrew Morton <akpm@linux-foundation.org>
To: Nelson Elhage <nelhage@ksplice.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm_release: Do a set_fs(USER_DS) before handling clear_child_tid.
Date: Tue, 30 Nov 2010 17:49:47 -0800 [thread overview]
Message-ID: <20101130174947.5ccc3778.akpm@linux-foundation.org> (raw)
In-Reply-To: <20101201005909.GC18995@ksplice.com>
On Tue, 30 Nov 2010 19:59:09 -0500 Nelson Elhage <nelhage@ksplice.com> wrote:
> On Tue, Nov 30, 2010 at 04:09:50PM -0800, Andrew Morton wrote:
> > On Mon, 29 Nov 2010 21:19:16 -0500
> > Nelson Elhage <nelhage@ksplice.com> wrote:
> >
> > > + * exited while inside set_fs(KERNEL_DS) for
> > > + * some reason (e.g. on a BUG()).
> > > */
> > > + set_fs(USER_DS);
> > > put_user(0, tsk->clear_child_tid);
> > > sys_futex(tsk->clear_child_tid, FUTEX_WAKE,
> > > 1, NULL, NULL, 0);
> >
> > Confused. The user can only exploit the wrong addr_limit if control
> > returns to userspace for the user's code to execute. But that won't be
> > happening, because this thread will unconditionally exit.
>
> The user can exploit the wrong addr_limit on the very next line, with the
> put_user() there. clear_child_tid is not checked in any way before this
> point. Writing a single zero might not seem like much, but it's enough for
> privilege escalation (e.g. overwrite the top half of a function pointer to point
> to userspace).
Ah, OK. Doh.
> I have a PoC code that uses this bug, along with CVE-2010-3849, to write a zero
> to an arbitrary kernel address, so I've tested that this is not theoretical.
>
> That's also why I put the set_fs() hidden inside mm_release, since that's the
> only place where (to my knowledge) it matters.
>
> On re-reading, I didn't mention clear_child_tid anywhere in the commit message,
> which was an error on my part, and explains the confusion. Sorry about that, and
> I hope this clears that up.
>
> Let me know if this makes more sense, and I'll send a revised patch.
I do think it would be better to fix up the addr_limit somewhere within
the oops code rather than in the regular code. Presumably just before
calling do_exit(). Isn't that the logical place? Plus it fixes up any
other such problems, whether they be there now or in the future.
Although that involves altering every arch, each in multiple places.
ick. Maybe at the start of do_exit()?
next prev parent reply other threads:[~2010-12-01 1:50 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-30 2:19 [PATCH] mm_release: Do a set_fs(USER_DS) before handling clear_child_tid Nelson Elhage
2010-12-01 0:09 ` Andrew Morton
2010-12-01 0:59 ` Nelson Elhage
2010-12-01 1:49 ` Andrew Morton [this message]
2010-12-01 2:27 ` [PATCH v2] do_exit(): Make sure we run with get_fs() == USER_DS Nelson Elhage
2010-12-01 2:50 ` KOSAKI Motohiro
2010-12-02 0:30 ` Andrew Morton
2010-12-02 0:48 ` KOSAKI Motohiro
2010-12-02 1:12 ` Andrew Morton
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=20101130174947.5ccc3778.akpm@linux-foundation.org \
--to=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nelhage@ksplice.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox