Re: [PATCH 4/4] test_printf: test printf family at runtime

[Rasmus Villemoes <linux@rasmusvillemoes.dk>]

On Tue, Sep 29 2015, Kees Cook <keescook@chromium.org> wrote:

> On Tue, Sep 29, 2015 at 12:10 AM, Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
>
>> I guess I could, but do we really want to intentionally trigger
>> WARN_ON_ONCEs? Say some distro chooses to load this module at boot
>> time, then we'd both spam the kernel log with "false positives", and
>> we'd have effectively disabled the WARN_ON_ONCEs for the actual
>> kernel code.
>
> Distros don't tend to run the test modules by default. The most common
> case is that it's part of a selftests run, in which case the machine
> has usually been freshly booted, etc. I think it's more important to
> catch regressions.
>
>> Maybe we can hide such things behind some module parameter, so that the
>> user explicitly has to ask for them. Also, we can't really probe the
>> "success" if these sanity checks from within the module (can we?) - the
>> user would have to check dmesg manually anyway.
>
> I think it's best that tests run with as few options as possible.
> Surely we can test the behavior? The bstr returns 0, so the string
> should be truncated? I haven't looked closely, but it seemed testable.

Well, yes, obviously we can check that part, but I also think it would
be nice to check that it actually resulted in a warning, which is what I
think would require manual inspection.

I'm still not convinced intentionally triggering a WARN on module load
is a good idea, even if the module wouldn't be loaded by normal
distros. Especially because of the _ONCE part, so that actual bugs might
not be warned about for the rest of that boot's lifetime. I'd certainly
like to hear what others think about this.

>>> I love tests! Thank you. :) One suggestion would be to wire it up to
>>> the tools/testing/selftests tree; it should be trivial once you change
>>> the test_printf_init return code.
>>
>> I'll look into that. Not sure I have too much time to work on this this
>> side of the merge window, and since these all seem to be things that can
>> be incrementally added, I'd prefer seeing something go into 4.4 instead
>> of waiting till it's "perfect". So unless I hear otherwise, I'll post a
>> v2 with the minor things addressed and ask Andrew to take that through
>> -mm.
>
> I'll send the glue patch...

Thanks. v2 coming up. For now I'll just change the return code; we can
always add tests for the sanity checks later, when we figure out the
best way to do them.

Rasmus