linux-kernel - Re: Rust kernel policy

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: <20250219160723.GB11480@pendragon.ideasonboard.com>
Date: Wed, 19 Feb 2025 18:07:23 +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:56:17PM +0100, Willy Tarreau wrote:
> On Wed, Feb 19, 2025 at 10:46:03AM -0500, James Bottomley wrote:
> > > > > 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.
> > 
> > Who said anything about not reference counting?
> 
> Nobody, but it was not said either that they were used at all!
> 
> >  One the things the
> > _cleanup_X annotations can do is drop references (or even locks).
> 
> OK then!
> 
> > >  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.
> > 
> > Well we could all speculate about the mess we'll make with any new
> > tool.  All I'm saying is that another project with a large code base
> > (systemd), which you can go an look at, managed to use these
> > annotations very successfully to simplify their error legs. Perhaps
> > there are reasons why the kernel can't be as successful, but I think
> > assuming failure from the outset isn't the best way to flush these
> > reasons out.
> 
> I'm not trying to assume failure or anything, just saying that it's
> probably not always as simple as calling kfree() on anything locally
> allocated for error paths to be magically cleaned, and it actually is
> more subtle (and Laurent confirmed my concerns illustrating that this
> case is precisely covered in glib using transfer of ownership).
> 
> And the temptation to return from everywhere since it's the only
> required statement (instead of a goto to a collecting place) becomes
> great and should sometimes be resisted to.
> 
> Regardless I do understand how these cleanups can help in a number of
> case, at least to avoid some code duplication.

They're particularly useful to "destroy" local variables that don't need
to be returned. This allows implementing scope guards, to facilitate
lock handling for instance. Once a mutex guard is instantiated, the
mutex is locked, and it will be guaranteed to be unlocked in every
return path.

-- 
Regards,

Laurent Pinchart