On Tue, Sep 29 2015, Kees Cook <keesc...@chromium.org> wrote: > On Tue, Sep 29, 2015 at 12:10 AM, Rasmus Villemoes > <li...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/