Re: Rust kernel policy

[Willy Tarreau <w@1wt.eu>]

Hi James,

On Wed, Feb 19, 2025 at 10:15:00AM -0500, James Bottomley wrote:
> On Wed, 2025-02-19 at 09:46 -0500, Martin K. Petersen wrote:
> > 
> > James,
> > 
> > > Could we possibly fix a lot of this by adopting the _cleanup_
> > > annotations[1]? I've been working in systemd code recently and they
> > > seem to make great use of this for error leg simplification.
> > 
> > We already have this:
> > 
> >   include/linux/cleanup.h
> > 
> > I like using cleanup attributes for some error handling. However, I'm
> > finding that in many cases I want to do a bit more than a simple
> > kfree(). And at that point things get syntactically messy in the
> > variable declarations and harder to read than just doing a classic
> > goto style unwind.
> 
> So the way systemd solves this is that they define a whole bunch of
> _cleanup_<type>_ annotations which encode the additional logic.  It
> does mean you need a globally defined function for each cleanup type,
> but judicious use of cleanup types seems to mean they only have a few
> dozen of these.

I may be missing something obvious, but this seems super dangerous to
me to perform lightly without reference counting, as it increases the
risks of use-after-free and double-free in case one of the allocated
objects in question can sometimes be returned. Users of such mechanisms
must be extremely cautious never to ever return a pointer derivated
from a variable tagged as such, or to properly NULL-assign the original
object for it not to double-free. So it might in the end require to be
careful about null-setting on return instead of explicitly freeing what
was explicitly allocated. I'm not sure about the overall benefit. Also
I suspect it encourages to multiply the return points, which makes it
even more difficult to possibly fix what needs to be fixed without
coming from a locally allocated variable (e.g. restore a state in a
parser etc). Maybe it's just me not seeing the whole picture, but as
a general case I prefer to forget a free() call (worst case: memory
leak) than forget a foo=NULL that may result in a double free, and the
description here makes me think the latter might more easily happen.

Regards,
Willy