linux-kernel - Re: [RFC PATCH 4/8] btrfs: simplify function protections with guards

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: <20251113084323.GG13846@twin.jikos.cz>
Date: Thu, 13 Nov 2025 09:43:23 +0100
From: David Sterba <dsterba@...e.cz>
To: Gladyshev Ilya <foxido@...ido.dev>
Cc: Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>,
	linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 4/8] btrfs: simplify function protections with guards

On Wed, Nov 12, 2025 at 09:49:40PM +0300, Gladyshev Ilya wrote:
> Replaces cases like
> 
> void foo() {
>     spin_lock(&lock);
>     ... some code ...
>     spin_unlock(&lock)
> }
> 
> with
> 
> void foo() {
>     guard(spinlock)(&lock);
>     ... some code ...
> }
> 
> While it doesn't has any measurable impact,

There is impact, as Daniel mentioned elsewhere, this also adds the
variable on stack. It's measuable on the stack meter, one variable is
"nothing" but combined wherever the guards are used can add up. We don't
mind adding variables where needed, occasional cleanups and stack
reduction is done. Here it's a systematic stack use increase, not a
reason to reject the guards but still something I cannot just brush off.

> it makes clear that whole
> function body is protected under lock and removes future errors with
> additional cleanup paths.

The pattern above is the one I find problematic the most, namely in
longer functions or evolved code. Using your example as starting point
a followup change adds code outside of the locked section:

void foo() {
    spin_lock(&lock);
    ... some code ...
    spin_unlock(&lock)

    new code;
}

with

void foo() {
    guard(spinlock)(&lock);
    ... some code ...

    new code;
}

This will lead to longer critical sections or even incorrect code
regarding locking when new code must not run under lock.

The fix is to convert it to scoped locking, with indentation and code
churn to unrelated code to the new one.

Suggestions like refactoring to make minimal helpers and functions is
another unecessary churn and breaks code reading flow.