[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DG5HX5K2T371.3JGY5PQIV554K@garyguo.net>
Date: Tue, 03 Feb 2026 17:17:21 +0000
From: "Gary Guo" <gary@...yguo.net>
To: "Danilo Krummrich" <dakr@...nel.org>, "Greg Kroah-Hartman"
<gregkh@...uxfoundation.org>
Cc: "Matthew Maurer" <mmaurer@...gle.com>, "Bjorn Andersson"
<andersson@...nel.org>, "Konrad Dybcio" <konradybcio@...nel.org>, "Satya
Durga Srinivasu Prabhala" <satyap@...cinc.com>, "Miguel Ojeda"
<ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "Daniel Almeida"
<daniel.almeida@...labora.com>, "Rafael J. Wysocki" <rafael@...nel.org>,
"David Airlie" <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>,
"Michal Wilczynski" <m.wilczynski@...sung.com>, "Dave Ertman"
<david.m.ertman@...el.com>, "Ira Weiny" <ira.weiny@...el.com>, "Leon
Romanovsky" <leon@...nel.org>, "Trilok Soni" <tsoni@...cinc.com>,
<linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
<rust-for-linux@...r.kernel.org>, <driver-core@...ts.linux.dev>,
<dri-devel@...ts.freedesktop.org>, <linux-pwm@...r.kernel.org>
Subject: Re: [PATCH v2 6/6] soc: qcom: socinfo: Convert to Rust
On Tue Feb 3, 2026 at 4:56 PM GMT, Danilo Krummrich wrote:
> On Tue Feb 3, 2026 at 5:48 PM CET, Greg Kroah-Hartman wrote:
>> On Tue, Feb 03, 2026 at 05:35:24PM +0100, Danilo Krummrich wrote:
>>> On Tue Feb 3, 2026 at 5:28 PM CET, Greg Kroah-Hartman wrote:
>>> > On Tue, Feb 03, 2026 at 03:46:35PM +0000, Matthew Maurer wrote:
>>> >> +impl Smem {
>>> >> + pub(crate) fn access<'a>(&'a self, dev: &'a Device<Bound>) -> Option<&'a Mmio> {
>>> >> + if *dev != *self.dev {
>>> >
>>> > How can this ever happen?
>>>
>>> You are right, since this resource is local to the driver, it should not be
>>> possible to have another devce from somewhere else (especially not a
>>> &Device<Bound>).
>>>
>>> However, from a Rust perspective I think not having this check would be unsound,
>>> as the method by itself would not be able guarantee correct behavor anymore.
>>>
>>
>> I don't really understand this last sentence, sorry. If this is "not
>> possible" why would that at the same time be "unsound"?
>
> It would be considered unsound because the function itself can not guarantee
> that it does not produce undefined behavior, i.e. it would otherwise becomes
> unsafe.
>
> But I'm not an expert on this, I'll let Benno and Gary jump in.
Yes, in this code, `Smem::access` is using the passed-in `&'a Device<Bound>` as
a proof that the device is still bound.
If the check is not present, the user of this API can give in a reference of a
different device, and now you're pontentially providing access to the MMIO
region to a device that is not bound anymore. A important property we want to
enforce is that UB cannot happen no matter how an API is misused (it may still
oops, which is bad, but better than UB).
This particular code, though, probably should use `Devres` as Danilo suggested.
PS. Personally I think this check (and the one in Devres::access) should use
assertion rather than `EINVAL`, because it is a bad driver bug when this is hit,
and adding failing code path for something that is a bug is something I really
dislike... It should at least be a `WARN`.
Best,
Gary
Powered by blists - more mailing lists