From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754810AbbI3JFZ (ORCPT ); Wed, 30 Sep 2015 05:05:25 -0400 Received: from mail-la0-f49.google.com ([209.85.215.49]:33310 "EHLO mail-la0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753415AbbI3JFT (ORCPT ); Wed, 30 Sep 2015 05:05:19 -0400 From: Rasmus Villemoes To: Kees Cook Cc: Andrew Morton , Tejun Heo , Andy Shevchenko , LKML Subject: Re: [PATCH 4/4] test_printf: test printf family at runtime Organization: D03 References: <1443202865-25533-1-git-send-email-linux@rasmusvillemoes.dk> <1443202865-25533-5-git-send-email-linux@rasmusvillemoes.dk> <871tdhsnsu.fsf@rasmusvillemoes.dk> X-Hashcash: 1:20:150930:andriy.shevchenko@linux.intel.com::rAdQOlHYH6My3Bcd:000000000000000000000000000015jY X-Hashcash: 1:20:150930:keescook@chromium.org::yEYdcCz47MtYIQCo:00000000000000000000000000000000000000001cEo X-Hashcash: 1:20:150930:akpm@linux-foundation.org::NzUWyClmr35GFMX8:00000000000000000000000000000000000052/E X-Hashcash: 1:20:150930:linux-kernel@vger.kernel.org::pMfPsXlzLDfwJUDJ:0000000000000000000000000000000004s6Q X-Hashcash: 1:20:150930:tj@kernel.org::NAze93GEkgNwSUlR:0000F6Ur Date: Wed, 30 Sep 2015 11:05:15 +0200 In-Reply-To: (Kees Cook's message of "Tue, 29 Sep 2015 10:32:16 -0700") Message-ID: <87d1x0l1kk.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 29 2015, Kees Cook wrote: > On Tue, Sep 29, 2015 at 12:10 AM, Rasmus Villemoes > 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