linux-kernel - Re: [PATCH v2 00/19] Implement DWARF modversions

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: <20240828225302.GH2130480@google.com>
Date: Wed, 28 Aug 2024 22:53:02 +0000
From: Sami Tolvanen <samitolvanen@...gle.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Luis Chamberlain <mcgrof@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Matthew Maurer <mmaurer@...gle.com>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Gary Guo <gary@...yguo.net>, Petr Pavlu <petr.pavlu@...e.com>,
	Neal Gompa <neal@...pa.dev>, Hector Martin <marcan@...can.st>,
	Janne Grunau <j@...nau.net>, Asahi Linux <asahi@...ts.linux.dev>,
	linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-modules@...r.kernel.org, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 00/19] Implement DWARF modversions

Hi Masahiro,

On Wed, Aug 28, 2024 at 04:04:09PM +0900, Masahiro Yamada wrote:
> On Fri, Aug 16, 2024 at 2:39 AM Sami Tolvanen <samitolvanen@...gle.com> wrote:
> >
> > The first 16 patches of this series add a small tool for computing
> 
> 
> Splitting a new tool into small chunks makes line-by-line review difficult.

I split this into smaller pieces to make it less of a chore to
review, but I'm also happy to squash these into larger patches if you
prefer. How would you like to see this split instead?

> For example, 02/19 adds malloc().
> 
> 03/19 immediately replaces it with calloc().
> 
> Then, I wonder why you did not add calloc() in the first place.

Yes, that wasn't ideal, but like I said in my other response, I tried
to keep the churn minimal. Please let me know if you spot any other
annoyances.

> And, I do not think it is so "small".
> It is bigger than the current genksyms.

In my defense, the first version was smaller, but sure, I'll drop the
false advertising from the cover letter now that it has more features.

> > symbol versions from DWARF, called gendwarfksyms. When passed a
> > list of exported symbols and an object file,
> 
> 
> Why is "a list of exported symbols" passed separately?
> 
> All necessary information is available in the object file.
> (The export symbols are listed in the .export_symbol section.

Unfortunately this is not the case for Rust object files where exports
are handled separately. Passing the list of symbols as input feels
more flexible to me, and also is rather convenient for debugging.

> > - Added a --symtypes flag for generating a genksyms-style
> >   symtypes output based on Petr's feedback, and refactored
> >   symbol version calculations to be based on symtypes instead
> >   of raw --dump-dies output.
> 
> 
> 
> I do not know if this is worthwhile.

Greg, Petr, do you want to comment on the usefulness of the symtypes
output? I was under the impression it was a useful tool for figuring
out exactly what caused the versions to change?

> And, it is obviously a build error.
> 
> gendwarfksyms cannot create %.symtypes from %.c.

Ah, this obviously needs to depends on the .o files instead. I'll sort
this out in v3.

Thanks for taking a look!

Sami