[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250219154504.GA11480@pendragon.ideasonboard.com>
Date: Wed, 19 Feb 2025 17:45:04 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Willy Tarreau <w@....eu>
Cc: James Bottomley <James.Bottomley@...senPartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Dan Carpenter <dan.carpenter@...aro.org>,
Christoph Hellwig <hch@...radead.org>,
Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
rust-for-linux <rust-for-linux@...r.kernel.org>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Greg KH <gregkh@...uxfoundation.org>,
David Airlie <airlied@...il.com>, linux-kernel@...r.kernel.org,
ksummit@...ts.linux.dev
Subject: Re: Rust kernel policy
On Wed, Feb 19, 2025 at 04:33:50PM +0100, Willy Tarreau wrote:
> 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.
Correct. That's how glib-based code works too. See
https://manpagez.com/html/glib/glib-2.56.0/glib-Memory-Allocation.php#g-steal-pointer
I don't know if there are static checkers (or compile-time checkers)
that catch or could catch direct returns.
> 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,
Laurent Pinchart
Powered by blists - more mailing lists