linux-kernel - Re: [PATCH] sched.h: drop in_ubsan field when UBSAN is in trap mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABvBcwY2FLJxc80Leibv=zZ-e_YbjkE+ZBH2LNoFy8HAeD8m6Q@mail.gmail.com>
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