linux-kernel - Re: [PATCH v6 3/4] vmalloc: Add debugfs modfraginfo

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: <1537815554.19013.49.camel@intel.com>
Date:   Mon, 24 Sep 2018 18:58:44 +0000
From:   "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To:     "keescook@...omium.org" <keescook@...omium.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        "jannh@...gle.com" <jannh@...gle.com>,
        "arjan@...ux.intel.com" <arjan@...ux.intel.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "x86@...nel.org" <x86@...nel.org>,
        "kristen@...ux.intel.com" <kristen@...ux.intel.com>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        "Hansen, Dave" <dave.hansen@...el.com>
Subject: Re: [PATCH v6 3/4] vmalloc: Add debugfs modfraginfo

On Fri, 2018-09-21 at 11:56 -0700, Kees Cook wrote:
> On Thu, Sep 13, 2018 at 2:31 PM, Rick Edgecombe
> <rick.p.edgecombe@...el.com> wrote:
> > +done:
> > +       gap = (MODULES_END - last_end);
> > +       if (gap > largest_free)
> > +               largest_free = gap;
> > +       total_free += gap;
> > +
> > +       spin_unlock(&vmap_area_lock);
> > +
> > +       seq_printf(m, "\tLargest free space:\t%lu kB\n", largest_free /
> > 1024);
> > +       seq_printf(m, "\t  Total free space:\t%lu kB\n", total_free / 1024);
> > +
> > +       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_enabled())
> > +               seq_printf(m, "Allocations in backup area:\t%lu\n",
> > backup_cnt);
> I don't think the IS_ENABLED is needed here?
The reason for this is that for ARCH=um, CONFIG_X86_64 is defined but
kaslr_enabled is not. kaslr_enabled is declared above to protect against a
compiler error.

So IS_ENABLED(CONFIG_RANDOMIZE_BASE) is protecting kaslr_enabled from causing a
linker error. It gets constant evaluated to 0 and the compiler optimizes out the
kaslr_enabled call. Thought it was better to guard with CONFIG_RANDOMIZE_BASE
than with CONFIG_UM, to try to catch the broader situation. I guess I could move
it to a helper inside ifdefs instead. Was trying to keep the ifdef-ed code down.

> I wonder if there is a better way to arrange this code that uses fewer
> ifdefs, etc. Maybe a single CONFIG that capture whether or not
> fine-grained module randomization is built in, like:
> 
> config RANDOMIZE_FINE_MODULE
>     def_bool y if RANDOMIZE_BASE && X86_64
> 
> #ifdef CONFIG_RANDOMIZE_FINE_MODULE
> ...
> #endif
> 
> But that doesn't capture the DEBUG_FS and PROC_FS bits ... so ...
> maybe not worth it. I guess, either way:
Hmmm, didn't know about that. Would clean it up some at least.

I wish the debugfs info could be in module.c to help with this IFDEFs, but it
needs vmalloc internals. MODULES_VADDR is not standardized across the ARCH's as
well, so this was my best attempt to implement this without having to make
changes in other architectures.
> Reviewed-by: Kees Cook <keescook@...omium.org>
> 
> -Kees
>