[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 11 Sep 2020 16:15:09 +0100
From: Elena Petrova <lenaptr@...gle.com>
To: Jann Horn <jannh@...gle.com>
Cc: Kernel Hardening <kernel-hardening@...ts.openwall.com>,
kernel list <linux-kernel@...r.kernel.org>, Kees Cook <keescook@...omium.org>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] sched.h: drop in_ubsan field when UBSAN is in trap mode
Hi Jann,
On Thu, 10 Sep 2020 at 20:35, Jann Horn <jannh@...gle.com> wrote:
>
> On Thu, Sep 10, 2020 at 3:48 PM Elena Petrova <lenaptr@...gle.com> wrote:
> > in_ubsan field of task_struct is only used in lib/ubsan.c, which in its
> > turn is used only `ifneq ($(CONFIG_UBSAN_TRAP),y)`.
> >
> > Removing unnecessary field from a task_struct will help preserve the
> > ABI between vanilla and CONFIG_UBSAN_TRAP'ed kernels. In particular,
> > this will help enabling bounds sanitizer transparently for Android's
> > GKI.
>
> The diff looks reasonable to me, but I'm curious about the
> justification in the commit message:
>
> Is the intent here that you want to be able to build a module without
> CONFIG_UBSAN and load it into a kernel that is built with
> CONFIG_UBSAN? Or the inverse?
The former. But more precisely, with GKI Google gives a promise, that
when certain GKI is released, i.e. at 4.19, its ABI will never ever
change (or, perhaps only change with <next letter> Android release),
so vendor modules could have an independent development lifecycle. And
this patch, when backported, will help enable boundsan on kernels
where ABI has already been frozen.
> Does this mean that in the future, gating new exported functions, or
> new struct fields, on CONFIG_UBSAN (independent of whether
> CONFIG_UBSAN_TRAP is set) will break Android?
I don't understand what you mean here, sorry.
> If you really want to do this, and using alternatives to patch out the
> ubsan instructions is not an option, I wonder whether it would be more
> reasonable to at least add a configuration where CONFIG_UBSAN is
> enabled but the compiler flag is not actually set. Then you could
> unconditionally build that android kernel and its modules with that
> config option, and wouldn't have to worry about structure size issues,
> dependencies on undefined symbols and so on.
Such setup might be confusing for developers. We were considering
something similar: to keep the in_ubsan field regardless of the
CONFIG_UBSAN option. But since non-trap mode is unlikely to be used on
production devices due to size and performance overheads, I think it's
better to just get rid of an unused field, rather than balloon
task_struct.
Cheers,
*lenaptr
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.
