Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 6 Apr 2020 17:08:22 +0000
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Kees Cook <keescook@chromium.org>
Cc:     Vlastimil Babka <vbabka@suse.cz>,
        Iurii Zaikin <yzaikin@google.com>,
        linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
        linux-mm@kvack.org, Ivan Teterevkov <ivan.teterevkov@nutanix.com>,
        Michal Hocko <mhocko@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Matthew Wilcox <willy@infradead.org>,
        "Eric W . Biederman" <ebiederm@xmission.com>,
        "Guilherme G . Piccoli" <gpiccoli@canonical.com>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Christian Brauner <christian.brauner@ubuntu.com>
Subject: Re: [PATCH 1/3] kernel/sysctl: support setting sysctl parameters
 from kernel command line
Message-ID: <20200406170822.GE11244@42.do-not-panic.com>
References: <20200330115535.3215-1-vbabka@suse.cz>
 <20200330115535.3215-2-vbabka@suse.cz>
 <20200330224422.GX11244@42.do-not-panic.com>
 <287ac6ae-a898-3e68-c7d8-4c1d17a40db9@suse.cz>
 <20200402160442.GA11244@42.do-not-panic.com>
 <202004021017.3A23B759@keescook>
 <20200402205932.GM11244@42.do-not-panic.com>
 <202004031654.C4389A04EF@keescook>
 <20200406140836.GA11244@42.do-not-panic.com>
 <202004060856.6BC17C5C99@keescook>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202004060856.6BC17C5C99@keescook>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Apr 06, 2020 at 08:58:50AM -0700, Kees Cook wrote:
> On Mon, Apr 06, 2020 at 02:08:36PM +0000, Luis Chamberlain wrote:
> > > Yes. Doing an internal extension isn't testing the actual code.
> > 
> > But it would.
> > 
> > [...]
> > > I don't think anything is needed for this series. It can be boot tested
> > > manually.
> > 
> > Why test it manually when it could be tested automatically with a new kconfig?
> 
> So, my impression is that adding code to the internals to test the
> internals isn't a valid test (or at least makes it fragile) because the
> test would depend on the changes to the internals (or at least depend on
> non-default non-production CONFIGs).

The *internal* aspect here is an extension to boot params under a
kconfig which would simply append to it, as if the user would have
added some more params. Since we already have test sysctl params the
only one we'd need to add on the test driver would be a dummy one which
tests the alias, on the second patch. We should have enough sysctls to
already test dummy values.

Nothing else would be needed as the sysctl test driver would just need
to test that the values expected when this is enabled is set.

> Can you send a patch for what you think this should look like? Perhaps
> I'm not correctly imagining what you're describing?

I rather get the person involved in the changes to do the testing so
as they're the ones designing the feature. If however it is not clear
what I mean I'm happy to elaborate.

Vlastimil do you get what I mean?

> Regardless of testing, I think this series is ready for -mm.

I'm happy for it to go in provided we at least devise a follow up plan
for testing. Otherwise -- like other things, it won't get done.

  Luis