linux-kernel - Re: [PATCH v5 13/16] export

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zwmm1yN1udGuGn7T@bombadil.infradead.org>
Date: Fri, 11 Oct 2024 15:29:43 -0700
From: Luis Chamberlain <mcgrof@...nel.org>
To: Matthew Maurer <mmaurer@...gle.com>
Cc: masahiroy@...nel.org, ndesaulniers@...gle.com, ojeda@...nel.org,
	gary@...yguo.net, Alex Gaynor <alex.gaynor@...il.com>,
	rust-for-linux@...r.kernel.org, linux-kbuild@...r.kernel.org,
	linux-kernel@...r.kernel.org, neal@...pa.dev, marcan@...can.st,
	j@...nau.net, asahi@...ts.linux.dev, linux-modules@...r.kernel.org,
	Boqun Feng <boqun.feng@...il.com>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>
Subject: Re: [PATCH v5 13/16] export_report: Rehabilitate script

On Fri, Oct 11, 2024 at 03:22:54PM -0700, Matthew Maurer wrote:
> On Fri, Oct 11, 2024 at 3:13 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
> >
> > On Wed, Sep 25, 2024 at 11:38:28PM +0000, Matthew Maurer wrote:
> > > * modules.order has .o files when in a build dir, support this
> >
> > The commit log is not clear, is it that it's always had *.o files, and
> > you're adding them now, why? Why is the .ko search now removed?
> 
> The script was broken when I found it,

My point is that commit messages should describe all this.

> but it was a script that
> analyzed MODVERSIONS, so I tried to ensure it would still work with my
> changes.

OK yes please add this informaiton into the comit log.

Also expand on it as to to *why* this was not a critical issue.

> This necessitated rehabilitating it first. I did not touch
> `.modules.order` files, but they contained `.o` and so this script
> wouldn't run correctly.

Just elaborate on the commit log. We cannot guess what is happening, and
so bringing clarity into exactly what you are doing and *why* is helpful
to determine the impact of not applying this.

> > > * .mod.c source layout has changed,
> >
> > When, why did this change not happen at that time?
> 
> It was changed for readability [1] back in 2019. I assume the change
> did not happen at that time because this script is rarely run.

This is crucial information to include in the commit log. When *would*
you use it? So that use case was broken since 2019? What negative
effects has this implicated?

> If we'd
> prefer to discard this patch and ignore the script instead (or remove
> it?), that's fine.

The feedback is for you to improve the commit logs, not to tell you to
not do something. We cannot read your mind, and this smelled like a fix,
but without a true evaluation and documentation of the impact of not
having this in place. These things need to be thought out and written
in the comit log.

> 
> [1]: https://lore.kernel.org/all/20190909113423.2289-2-yamada.masahiro@socionext.com/
> 
> >
> > > update regexes to match
> >
> > Why did this not break anything before ? Is this fixing something, or
> > is it prep work?
> >
> > > * Add a stage 3, to be more robust against additional .mod.c content
> >
> > Future .mod.c changes?
> 
> The rest of this series adds additional `.mod.c` content to support
> the string names. This stage 3 is intended to prevent that from
> causing the script to choke.

The commit log should clarify all this.

  Luis