linux-kernel - Re: [PATCH v6 2/3] rust: io: mem: add a generic iomem 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Z6Ttn83DQ_8asHXl@cassiopeiae>
Date: Thu, 6 Feb 2025 18:13:03 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: 崔光博 <2407018371@...com>
Cc: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>,
	Alice Ryhl <aliceryhl@...gle.com>, daniel.almeida@...labora.com,
	a.hindborg@...nel.org, alex.gaynor@...il.com,
	benno.lossin@...ton.me, bjorn3_gh@...tonmail.mco,
	boqun.feng@...il.com, boris.brezillon@...labora.com,
	gary@...yguo.net, gregkh@...uxfoundation.org,
	linux-kernel@...r.kernel.org, ojeda@...nel.org, rafael@...nel.org,
	robh@...nel.org, rust-for-linux@...r.kernel.org, tmgross@...ch.edu
Subject: Re: [PATCH v6 2/3] rust: io: mem: add a generic iomem abstraction

On Fri, Feb 07, 2025 at 12:40:14AM +0800, 崔光博 wrote:
> 
> 
> > 2025年2月7日 00:11，Miguel Ojeda <miguel.ojeda.sandonis@...il.com> 写道：
> > 
> > On Thu, Feb 6, 2025 at 4:59 PM Guangbo Cui <2407018371@...com> wrote:
> >> 
> >> With CONFIG_RUST_BUILD_ASSERT_ALLOW=y enabled, this compilation succeeds.
> > 
> > Yes, that is expected too (but note that the config option is there
> > just in case -- it should not happen that it is needed in normal
> > builds).
> > 
> >> Even if the size is determined at compile time, the compilation will still fail
> >> if CONFIG_RUST_BUILD_ASSERT_ALLOW is not enabled.
> > 
> > Yes, that is expected -- the idea is that you cannot make the mistake
> > of calling those.
> > 
> > I think you are suggesting only exposing the methods in the case where
> > calling them would work? That would be great if a design allows for
> > it, of course.
> 
> Yes, if the methods could not work, we should not expose them.
> 
> > By the way, Daniel, in patch 3/3 there is this comment:
> > 
> > +    ///     // Unlike `ioremap_resource_sized`, here the size of the
> > memory region
> > +    ///     // is not known at compile time, so only the `try_read*`
> > and `try_write*`
> > +    ///     // family of functions are exposed, leading to runtime
> > checks on every
> > +    ///     // access.
> > 
> > Is the "only ... are exposed" correct? i.e. are they exposed? / is
> > this potentially confusing?
> 
> They are exposed. If size is not known at compile time, calling the `read`
> and `write` will never compile failed. Example:

That's two different things here. Miguel questions whether the comment is
correct. And I think it's not, they are indeed exposed.

> 
> ```C
> let raw_io: IoRaw<0> = IoRaw::new(0, 8)?;
> let io = unsafe { Io::from_raw(&raw_io) };
> io.writeb(0xff, 0xffff);
> ```
> If I make any mistakes, please correct me. Thanks!

This behavior is on purpose.

IoRaw::new() is equivalent to IoRaw::<0>::new(), which means that you set the
compile time validated size of the I/O region to zero.

Hence, calling writeb() fails, because every operation exeeds the boundary of
zero.

In your case the runtime boundary is 8, hence the following calls do succeed.

try_readb(0);
try_readb(7);

Whereas the following would fail on runtime.

try_readb(8);

> 
> Best regards,
> Guangbo Cui
>