linux-kernel - Re: [PATCH v2] locking/local_lock: s/l/__l/ and s/tl/_

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] [day] [month] [year] [list]

Message-ID: <5f7821d5-0b48-4600-ab99-d76a52361fc1@kernel.org>
Date: Sat, 27 Sep 2025 12:04:19 +0900
From: Vincent Mailhol <mailhol@...nel.org>
To: Vlastimil Babka <vbabka@...e.cz>, Shakeel Butt <shakeel.butt@...ux.dev>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Alexei Starovoitov <ast@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
 Boqun Feng <boqun.feng@...il.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] locking/local_lock: s/l/__l/ and s/tl/__tl/ to reduce
 risk of shadowing

On 9/26/25 11:16 PM, Vlastimil Babka wrote:
> +CC LOCKING PRIMITIVES maintainers. Looks like local_lock files were never
> added to the section, should we?
> 
> On 9/24/25 20:03, Vincent Mailhol wrote:
>> The Linux kernel coding style [1] advises to avoid common variable
>> names in function-like macros to reduce the risk of collisions.
> 
> I think it would be better if the tools like sparse could recognize if the
> shadowing happens inside a macro only and thus really unlikely to cause a
> misuse due to confusion (code thinks it's manipulating an outer instance but
> instead it's the inner one), because macros in their definition would never
> intend to manipulate a possible outer instance, right? Or are there any
> other problems due to shadowing besides this risk?

Thank would mean:

  - rewriting the shadowing check in sparse
  - removing the -Wshadow from the W=2 list
  - modifying the kernel coding style

I am not against this. But I am not unhappy with the current status quo either.

So far, I kept sending patches whenever I saw such shadow warning in header
files. And over the last five years, this resulted in only three occurrences:

  - commit 146034fed6ee ("x86/asm/bitops: Use __builtin_ffs() to evaluate
    constant expressions")
    Link: https://git.kernel.org/torvalds/c/146034fed6ee


  - commit 9ce02f0fc683 ("x86/bug: Prevent shadowing in __WARN_FLAGS")
    Link: https://git.kernel.org/torvalds/c/9ce02f0fc683

  - this patch

Between sending one patch every couple year or enrolling to a quest to modify
the tooling, my choice is already made. If someone else want to do this change,
I would be supportive, but that person will not be me.

On a side note, I want to highlight that it is not that I am reluctant to modify
the tooling. For example, I sent contributed this commit to sparse last week:

commit 366ad4b2fa3e ("Warn about "unsigned value that used to be signed against
zero"")

Link:
https://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git/commit/?id=366ad4b2fa3e

As anyone here, I choose my battles, and rewriting the shadow checks is not in
my list.

>> Throughout local_lock_internal.h, several macros use the rather common
>> variable names 'l' and 'tl'. This already resulted in an actual
>> collision: the __local_lock_acquire() function like macro is currently
>> shadowing the parameter 'l' of the:
>>
>>   class_##_name##_t class_##_name##_constructor(_type *l)
>>
>> function factory from linux/cleanup.h.
>>
>> Rename the variable 'l' to '__l' and the variable 'tl' to '__tl'
>> throughout the file to fix the current name collision and to prevent
>> future ones.
>>
>> [1] https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
>>
>> Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> 
> That said I don't oppose the change, but not my call.

Thanks!


Yours sincerely,
Vincent Mailhol