|
Message-ID: <87mvsa5q40.fsf@gamma.ozlabs.ibm.com>
Date: Wed, 13 Jan 2016 10:34:39 +1100
From: Daniel Axtens <dja@...ens.net>
To: Jann Horn <jann@...jh.net>, kernel-hardening@...ts.openwall.com, linux-kernel@...r.kernel.org
Cc: Andrew Morton <akpm@...ux-foundation.org>, HATAYAMA Daisuke <d.hatayama@...fujitsu.com>, Vitaly Kuznetsov <vkuznets@...hat.com>, Baoquan He <bhe@...hat.com>, Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Subject: Re: [RFC] kernel/panic: place an upper limit on number of oopses
Jann Horn <jann@...jh.net> writes:
> To prevent an attacker from turning a mostly harmless oops into an
> exploitable issue using a refcounter wraparound caused by repeated
> oopsing, limit the number of oopses.
>
> I have not experimentally verified whether the attack I describe
> in the comment works, but I don't see why it wouldn't.
> (f_count increments through fget() use atomic_long_inc_not_zero(),
> but get_file() just does a normal increment and is e.g.
> used by dup_fd().)
>
> This approach is strictly inferior to PAX_REFCOUNT, but as long
> as that's not upstreamed and turned on by default, it might make
> sense to at least use this patch.
>
> Opinions?
I'm torn between making the limit configurable and not adding to the
massive proliferation of config options.
Other comments below.
>
> Signed-off-by: Jann Horn <jann@...jh.net>
> ---
> kernel/panic.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 4b150bc..27a480d 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -422,9 +422,37 @@ void print_oops_end_marker(void)
> */
> void oops_exit(void)
> {
> + static atomic_t oops_counter = ATOMIC_INIT(0);
> +
> do_oops_enter_exit();
> print_oops_end_marker();
> kmsg_dump(KMSG_DUMP_OOPS);
> +
> + /*
> + * Every time the system oopses, if the oops happens while a
> + * reference to an object was held (e.g. in a VFS function),
> + * the reference leaks. If the oops doesn't also leak memory,
> + * repeated oopsing can cause the reference counter to wrap
> + * around - in particular, on 32bit systems, f_count in
> + * struct file is only 32 bits long and can realistically
> + * wrap around.
> + * This means that an oops, even if it's just caused by an
> + * unexploitable-looking NULL pointer dereference or so,
> + * could maybe be turned into a use-after-free through a
> + * counter overincrement, and a use-after-free might be
> + * exploitable.
> + * To reduce the probability that this happens, place an
> + * upper bound on how often the kernel may oops - after this
> + * limit is reached, just panic.
> + * The constant used as limit should be low enough to
> + * mitigate this kind of exploitation attempt, but high
> + * enough to avoid unnecessary panics.
> + */
> + if (atomic_inc_return(&oops_counter) >= 0x100000 &&
> + panic_on_oops == 0) {
Do you need to check panic_on_oops? If it was 1 you'd already have
paniced, right?
> + pr_emerg("oopsed too often, setting panic_on_oops=1\n");
> + panic_on_oops = 1;
Would it be easier to just panic here, rather than wait for another oops?
> + }
> }
>
> #ifdef WANT_WARN_ON_SLOWPATH
Regards,
Daniel
> --
> 2.1.4
Download attachment "signature.asc" of type "application/pgp-signature" (860 bytes)
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.