lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v6 3/4] vmalloc: Add debugfs modfraginfo
On Mon, Sep 24, 2018 at 11:58 AM, Edgecombe, Rick P
<rick.p.edgecombe@intel.com> wrote:
> 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@intel.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.

Ah yes, UM. Perhaps kaslr_enabled() could be defined somewhere so that
it would link sanely? (Maybe in module.h?)

>> 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.

Yeah, I've long wanted to try to sandardize the module+vmalloc guts,
but it's just different enough in each architecture that it eludes
people.

-Kees

--
Kees Cook
Pixel Security

\
 
 \ /
  Last update: 2018-09-24 22:04    [W:0.078 / U:0.228 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site