linux-kernel - Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to k[v]free_rcu

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: <20230316012516.GK860405@mit.edu>
Date:   Wed, 15 Mar 2023 21:25:16 -0400
From:   "Theodore Ts'o" <tytso@....edu>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Uladzislau Rezki <urezki@...il.com>,
        Jens Axboe <axboe@...nel.dk>,
        LKML <linux-kernel@...r.kernel.org>, RCU <rcu@...r.kernel.org>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>,
        Philipp Reisner <philipp.reisner@...bit.com>,
        Bryan Tan <bryantan@...are.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Bob Pearson <rpearsonhpe@...il.com>,
        Ariel Levkovich <lariel@...dia.com>,
        Julian Anastasov <ja@....bg>
Subject: Re: [PATCH 00/13] Rename k[v]free_rcu() single argument to
 k[v]free_rcu_mightsleep()

On Wed, Mar 15, 2023 at 06:08:19PM -0400, Joel Fernandes wrote:
> 
> I am doubtful there may be a future where it does not sleep. Why?
> Because you need an rcu_head *somewhere*.

I think the real problem was that this won't sleep:

   kfree_rcu(ptr, rhf);

While this *could* sleep:

   kfree_rcu(ptr);

So the the original sin was to try to make the same mistake that C++
did --- which is to think that it's good to have functions that have
the same name but different function signatures, and in some cases,
different semantic meanings because they have different implementations.

Personally, this is why I refuse to use C++ for any of my personal
projects --- this kind of "magic" looks good, but it's a great way to
potentially shoot yourself (or worse, your users) in the foot.

So separating out the two-argument kfree_rcu() from the one-argument
kfree_rcu(), by renaming the latter to something else is IMHO, a
Really F***** Good Idea.  So while, sure, kfree_rcu_mightsleep() might
be a little awkward, the name documents the potential landmind
involved with using that function, that's a good thing.  Because do
you really think users will always conscientiously check the
documentation and/or the implementation before using the interface?  :-)

If you hate that name, one other possibility is to try to use the
two-argument form kfree_rcu() and arrange to *have* a rcu_head in the
structure.  That's going to be better from a performance perspective,
and thus kinder to the end user than using rcu_synchronize().

Cheers,

					- Ted