linux-kernel - Re: [PATCH] fs: Use a cleanup attribute in copy

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: <878qhpc3ip.fsf@>
Date: Sun, 05 Oct 2025 07:37:50 +0200
From: Miquel Sabaté Solà <mssola@...ola.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: linux-fsdevel@...r.kernel.org,  brauner@...nel.org,
  linux-kernel@...r.kernel.org,  jack@...e.cz
Subject: Re: [PATCH] fs: Use a cleanup attribute in copy_fdtable()

Al Viro @ 2025-10-04 22:19 +01:

> On Sat, Oct 04, 2025 at 11:03:40PM +0200, Miquel Sabaté Solà wrote:
>> This is a small cleanup in which by using the __free(kfree) cleanup
>> attribute we can avoid three labels to go to, and the code turns to be
>> more concise and easier to follow.
>
> Have you tried to build and boot that?

Yes, and it worked on my machine...

>
> That aside, it is not easier to follow in that form - especially since
> kfree() is *not* the right destructor for the object in question.
> Having part of destructor done via sodding __cleanup, with the rest
> open-coded on various failure exits is confusing as hell.
>
> RAII has its uses, but applied unidiomatically it ends up being a mess
> that is harder to follow and reason about than the dreadful gotos it
> replaces.

I agree that it would generally not be the right destructor for it, but
in the case of this function it ends up being equivalent. But I see
that, if in general that wouldn't be the proper way, declaring the
fdtable variable like that can be misleading, even if equivalent
here. Thus, defeating the purpose of this patch.

>
> NAKed-by: Al Viro <viro@...iv.linux.org.uk>

Thanks for the review,
Miquel

Download attachment "signature.asc" of type "application/pgp-signature" (898 bytes)