Re: [PATCH 2/6] genalloc: selftest

[Igor Stoppa]

On 22/02/18 11:14, Igor Stoppa wrote:
> 
> 
> On 22/02/18 00:28, Kees Cook wrote:
>> On Tue, Feb 20, 2018 at 8:59 AM, Igor Stoppa <igor.stoppa@xxxxxxxxxx> wrote:
>>>
>>>
>>> On 13/02/18 01:50, Kees Cook wrote:
>>>> On Mon, Feb 12, 2018 at 8:52 AM, Igor Stoppa <igor.stoppa@xxxxxxxxxx> wrote:
> 
> [...]
> 
>>>>> +       genalloc_selftest();
>>>>
>>>> I wonder if it's possible to make this module-loadable instead? That
>>>> way it could be built and tested separately.
>>>
>>> In my case modules are not an option.
>>> Of course it could be still built in, but what is the real gain?
>>
>> The gain for it being a module is that it can be loaded and tested
>> separately from the final kernel image and module collection. For
>> example, Chrome OS builds lots of debugging test modules but doesn't
>> include them on the final image. They're only used for testing, and
>> can be separate from the kernel and "production" modules.
> 
> ok

I started to turn this into a module, but after all it doesn't seem like
it would give any real advantage, compared to the current implementation.

This testing is meant to catch bugs in memory management as early as
possible in the boot phase, before users of genalloc start to fail in
mysterious ways.

This includes, but is not limited to: MCE on x86, uncached pages
provider on arm64, dma on arm.

Should genalloc fail, it's highly unlikely that the test rig would even
reach the point where it can load a module and run it, even if it is
located in initrd.

The test would not be run, precisely at the moment where its output
would be needed the most, leaving a crash log that is hard to debug
because of memory corruption.

I do not know how Chrome OS builds are organized, but I imagine that
probably there is a separate test build, where options like lockdep,
ubsan, etc. are enabled.

All options that cannot be left enabled in a production kernel, but are
very useful for sanity checks and require a separate build.

Genalloc testing should be added there, rather than in a module, imho.

--
igor