linux-kernel - Re: [PATCH v2] module: Harden STRICT_MODULE

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 13 Aug 2020 15:07:13 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Jessica Yu <jeyu@...nel.org>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Szabolcs Nagy <szabolcs.nagy@....com>,
        Will Deacon <will@...nel.org>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Kees Cook <keescook@...omium.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>,
        Mark Rutland <mark.rutland@....com>, nd <nd@....com>
Subject: Re: [PATCH v2] module: Harden STRICT_MODULE_RWX

On Thu, 13 Aug 2020 at 15:04, Jessica Yu <jeyu@...nel.org> wrote:
>
> +++ Ard Biesheuvel [13/08/20 10:36 +0200]:
> >On Wed, 12 Aug 2020 at 22:00, Peter Zijlstra <peterz@...radead.org> wrote:
> >>
> >> On Wed, Aug 12, 2020 at 06:37:57PM +0200, Ard Biesheuvel wrote:
> >> > I know there is little we can do at this point, apart from ignoring
> >> > the permissions - perhaps we should just defer the w^x check until
> >> > after calling module_frob_arch_sections()?
> >>
> >> My earlier suggestion was to ignore it for 0-sized sections.
> >
> >Only they are 1 byte sections in this case.
> >
> >We override the sh_type and sh_flags explicitly for these sections at
> >module load time, so deferring the check seems like a reasonable
> >alternative to me.
>
> So module_enforce_rwx_sections() is already called after
> module_frob_arch_sections() - which really baffled me at first, since
> sh_type and sh_flags should have been set already in
> module_frob_arch_sections().
>
> I added some debug prints to see which section the module code was
> tripping on, and it was .text.ftrace_trampoline. See this snippet from
> arm64's module_frob_arch_sections():
>
>                 else if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE) &&
>                          !strcmp(secstrings + sechdrs[i].sh_name,
>                                  ".text.ftrace_trampoline"))
>                         tramp = sechdrs + i;
>
> Since Mauro's config doesn't have CONFIG_DYNAMIC_FTRACE enabled, tramp
> is never set here and the if (tramp) check at the end of the function
> fails, so its section flags are never set, so they remain WAX and fail
> the rwx check.

Right. Our module.lds does not go through the preprocessor, so we
cannot add the #ifdef check there currently. So we should either drop
the IS_ENABLED() check here, or simply rename the section, dropping
the .text prefix (which doesn't seem to have any significance outside
this context)

I'll leave it to Will to make the final call here.