linux-hardening - Re: [PATCH v2 00/10] lib/string

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: <202503041222.6A4843E77E@keescook>
Date: Tue, 4 Mar 2025 12:40:57 -0800
From: Kees Cook <kees@...nel.org>
To: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Cc: Andy Shevchenko <andy@...nel.org>, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 00/10] lib/string_choices: Add new helpers

On Tue, Mar 04, 2025 at 02:12:41AM +0000, Kuninori Morimoto wrote:
> I would like to use string_choices helper to cleanup the code, but it is
> missing some of well used string pair in kernel. This patch-set adds it.
> 
> Step1
> 	Add new helpers (This patch-set)
> Step2
> 	Each driver/framework use new helper

The Coccinelle rules are very easy to write. For example:

@str_input_output@
expression E;
@@

-       (E ? "input" : "output")
+       str_input_output(E)

@str_output_input@
expression E;
@@

-       (E ? "output" : "input")
+       str_output_input(E)

Then running those will generate the treewide patches. I wrote and ran
al of these, and the results for me were:

str_in_out.patch:              66 files changed, 130 insertions(+), 136 deletions(-)
str_tx_rx.patch:               33 files changed, 48 insertions(+), 53 deletions(-)
str_input_output.patch:        17 files changed, 27 insertions(+), 29 deletions(-)
str_enabling_disabling.patch:  16 files changed, 25 insertions(+), 25 deletions(-)
str_Y_N.patch:                 13 files changed, 98 insertions(+), 89 deletions(-)
str_kernel_user.patch:          7 files changed, 8 insertions(+), 7 deletions(-)
str_level_edge.patch:           5 files changed, 6 insertions(+), 6 deletions(-)
str_to_from.patch:              3 files changed, 4 insertions(+), 4 deletions(-)
str_pass_fail.patch:            2 files changed, 2 insertions(+), 2 deletions(-)
str_attach_detach.patch:        1 file changed, 4 insertions(+), 4 deletions(-)

So, IMO, the first 5 are clear winners. The others are probably fine,
but pass/fail and attach/detach are really not used much.

> One concern is that it adds "pass/fail", but we can find many similar
> strings, like below. I have choiced "pass/fail" and use it on all cases
> in my local branch (You can see some of them in [SAMPLE].
> I need your opinion
> 
> 	passed     / failed
> 	succeed    / failed
> 	success    / failed
> 	successful / failed
> 	succeeded  / failed
> 	worked     / failed

As Andy said, don't change any existing strings into something else.
However, of the above 6 styles, I'd say find the most common and do
those conversions.

>  244 files changed, 604 insertions(+), 602 deletions(-)

I only saw:

 161 files changed, 352 insertions(+), 355 deletions(-)

You have a lot in arch/ that I don't have, and when I spot-checked those
files, I don't see what from this series you were converted? Perhaps you
were doing more than those from this series (like "write"/"read")?

-Kees

-- 
Kees Cook