linux-kernel - Re: [PATCH] rust: regulator: add a bare minimum regulator abstraction

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z7cY5pr8DW92zt46@finisterre.sirena.org.uk>
Date: Thu, 20 Feb 2025 11:58:30 +0000
From: Mark Brown <broonie@...nel.org>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: lgirdwood@...il.com, sebastian.reichel@...labora.com,
	sjoerd.simons@...labora.co.uk, ojeda@...nel.org,
	alex.gaynor@...il.com, boqun.feng@...il.com, gary@...yguo.net,
	bjorn3_gh@...tonmail.com, a.hindborg@...nel.org,
	benno.lossin@...ton.me, aliceryhl@...gle.com, tmgross@...ch.edu,
	dakr@...nel.org, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: regulator: add a bare minimum regulator abstraction

On Thu, Feb 20, 2025 at 06:42:25AM -0300, Daniel Almeida wrote:

> >> I asked for a few opinions privately and was told that “if the C API prefers not to do that
> >> why should you?”

> > Well, the C API also doesn't ignore either enable or disable attempts...
> > the theory is that if the consumer messed up it's safer to not power the
> > hardware off suddenly when something might not have been cleaned up.
> > The general approach the API takes is to only take actions it's been
> > explicitly asked to do, that way we're not hard coding anything that
> > causes trouble for consumers and since we need constraints to enable any
> > action that gets taken we're less likely to have default behaviour
> > causing hardware damage somehow.  If we think we've lost track of the
> > reference counting we just scream about it but don't try to take
> > corrective action.

> So, are you OK with this approach? i.e.:

> > ```
> >  fn drop(&mut self) {
> > 
> >    while self.enabled_count > 0 {
> > 
> >            if let Err(e) = self.disable() {
> >              break;
> >            }
> >        }
> >  }
> > ```

> Where `enable()` increments self.enable_count and `disable()` decrements it.

It's not ideal since it'll just silently clean up any enables that were
done, ideally the Rust bindings would prevent leaks of enables like they
are intended to prevent memory leaks and this wouldn't come up at all.
This seems less safe than the C API since we don't even detect any
possible isues, we'll just silently hide leaks.

> >>> Perhaps an enable should be an object that's allocated and carried about
> >>> by whatever's holding the reference, I don't know if that plays nicely
> >>> with how Rust likes to ensure safety with this stuff?

> >> As I said, this doesn’t work very well, unless someone corrects my reasoning on a

> > I don't think I saw the previous mail?

> You didn’t get this?

> https://lore.kernel.org/rust-for-linux/Z7aHQBYXZ5jlGRte@finisterre.sirena.org.uk/T/#m9348ad4fdc056d7f6d0bfec6529d4c80afdcd335

Don't think so (but the copy I'm getting from the archive has mangled
formatting with weird line lengths, it's possible that it got hit by
spam filtering or I discarded it manually as junk due to the
formatting).  I'm not sure I understand the concerns raised there, but
it's possible Rust is less helpful for this stuff than I had imagined,
or that the whole thing with layering Rust on top of the C code is
getting in the way of whatever idioms fully native Rust code would use
to avoid issues.  I'll reply there...

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