[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87d1x0l1kk.fsf@rasmusvillemoes.dk>
Date: Wed, 30 Sep 2015 11:05:15 +0200
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Kees Cook <keescook@...omium.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Tejun Heo <tj@...nel.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] test_printf: test printf family at runtime
On Tue, Sep 29 2015, Kees Cook <keescook@...omium.org> wrote:
> On Tue, Sep 29, 2015 at 12:10 AM, Rasmus Villemoes
> <linux@...musvillemoes.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists