linux-kernel - Re: thoughts wanted on dead code hunting?

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: <202405141706.1D5D26C@keescook>
Date: Tue, 14 May 2024 17:14:27 -0700
From: Kees Cook <keescook@...omium.org>
To: "Dr. David Alan Gilbert" <linux@...blig.org>
Cc: linux-kernel@...r.kernel.org, nathan@...nel.org,
	luc.vanoostenryck@...il.com
Subject: Re: thoughts wanted on dead code hunting?

On Thu, May 09, 2024 at 12:08:56PM +0000, Dr. David Alan Gilbert wrote:
>   That's found me ~200 candidates; where I guess 150ish are probably
> real; but my hacky script is, well trivial and hacky, so they each
> need eyeballing, then a git lookup to see why they're unused, and a
> compile just to make there's not some subtle macro somewhere.

Nice finds! People are usually big fans of code removal patches. :)

>  ** Questions:
>   a) Can anyone think of a better tool than my script (see bottom)?
>    The simplicity is a blessing & a curse - it doesn't know about
>    #ifdef's so I don't need to try lots of configs, but at the same
>    time, it can't tell if the struct actually gets used in a macro
>    and I have to eyeball for a struct which is assigned to as
>    a variable at declaration time.

I'm not sure I've seen anything better.

I tend to use stuff like Coccinelle (spatch) for finding specific struct
usage, but it can sometimes be slow when trying to process headers
recursively. e.g.:

// Options: --recursive-includes
@find@
struct to_be_removed INSTANCE;
struct to_be_removed *POINTER;

(
*       INSTANCE
|
*       POINTER
)


(I bet this could be improved, but it should be a usable example.)

So this might very a given struct isn't used.

>   b) The dead structs are all over; so they've mostly been individual
>   patches rather than a big patch series - how do people feel about
>   another 150ish similar patches ?

Generally the smaller patches are preferred. For this kind of thing,
though, I'd probably collect them by individual header files, rather
than one-patch-per-struct.

If you have one giant patch, this tool can help break it up into
per-subsystem patches (it isn't perfect, but does its best):
https://github.com/kees/kernel-tools/blob/trunk/split-on-maintainer

>   * There's a few cases where people have added 'static' to a variable
>    to cleanup compiler warnings, but actually they just needed to
>    delete the variable.

Hah. Yeah, these are nice to find and remove.

>   * A harder problem is unused structure members; some I've spotted
>    by accident, some follow from what else I delete; e.g. if you
>    delete a LIST_HEAD, there's a good chance there's a struct somewhere
>    with the list entry in it that's no longer used.

This is especially tricky because a giant amount of structs in the
kernel actually describe over-the-wire or on-hardware structures that
maybe the kernel doesn't care about all the members, but they're still
needed to keep the layout correct.

-Kees

-- 
Kees Cook