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: <20250922112922.0f57fbf3@eugeo>
Date: Mon, 22 Sep 2025 11:29:22 +0100
From: Gary Guo <gary@...yguo.net>
To: "Benno Lossin" <lossin@...nel.org>
Cc: "Joel Fernandes" <joelagnelf@...dia.com>,
 <linux-kernel@...r.kernel.org>, "Miguel Ojeda" <ojeda@...nel.org>, "Alex
 Gaynor" <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>,
 Björn Roy Baron <bjorn3_gh@...tonmail.com>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 <acourbot@...dia.com>, "Alistair Popple" <apopple@...dia.com>, "Timur Tabi"
 <ttabi@...dia.com>, <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH] rust: print: Fix issue with rust_build_error

On Sun, 21 Sep 2025 09:12:45 +0200
"Benno Lossin" <lossin@...nel.org> wrote:

> On Sun Sep 21, 2025 at 2:45 AM CEST, Joel Fernandes wrote:
> > On Sat, Sep 20, 2025 at 10:09:30PM +0200, Benno Lossin wrote:  
> >> On Sat Sep 20, 2025 at 6:19 PM CEST, Joel Fernandes wrote:  
> >> > When printing just before calling io.write32(), modpost fails due to
> >> > build_assert's missing rust_build_error symbol. The issue is that, the
> >> > printk arguments are passed as reference in bindings code, thus Rust
> >> > cannot trust its value and fails to optimize away the build_assert.
> >> >
> >> > The issue can be reproduced with the following simple snippet:
> >> >   let offset = 0;
> >> >   pr_err!("{}", offset);
> >> >   io.write32(base, offset);  
> >> 
> >> I took a long time to understand this and I think I got it now, but
> >> maybe I'm still wrong: passing `offset` to `printk` via a reference
> >> results in the compiler thinking that the value of `offset` might be
> >> changed (even though its a shared reference I assume). For this reason
> >> the `build_assert!` used by `io.write32` cannot be optimized away.  
> >
> > Yes, that's correct and that's my understanding. I in fact also dumped the IR
> > and see that the compiler is trying to reload the pointer to the offset. I
> > feel this is a compiler bug, and for immutable variables, the compiler should
> > not be reloading them even if they are passed to external code? Because if
> > external code is modifying immutable variables, that is buggy anyway.  
> 
> It would be great if you could add all of the extra info that you looked
> at into the commit message here. So all of the code to reproduce the
> issue, the IR you looked at...
> 
> >> > Fix it by just using a closure to call printk. Rust captures the
> >> > arguments into the closure's arguments thus breaking the dependency.
> >> > This can be fixed by simply creating a variable alias for each variable
> >> > however the closure is a simple and concise fix.  
> >> 
> >> I don't think this is the fix we want to have.  
> >
> > Yeah its ugly, though a small workaround. IMO ideally the fix should be in
> > the compiler. I want to send a proposal for this in fact. I looked at the MIR
> > and I believe with my limited understanding, that the MIR should be issuing a
> > copy before making a pointer of the immutable variable if pointers to
> > immutable variables are being passed to external functions.  
> 
> ... and definitely the MIR.
> 
> > If I were to send a proposal to the Rust language to start a discussion
> > leading to a potential RFC stage, would you mind guiding me on doing so?  
> 
> I agree that this should be fixed in the compiler if we aren't missing
> some attribute on one of our functions.
> 
> Note that this probably doesn't require an RFC, since it's not a big
> feature and I imagine that there isn't much controversy about it. We can
> mention this in our sync meeting on Wednesday with the Rust folks & see
> what they have to say about it. After that we probably want to open an
> issue about it, I'll let you know.
> 
> >> In fact it already
> >> doesn't compile all of the existing code:  
> >
> > Oh gosh, I should have run doctests. I just did a normal build. Let me see
> > if I can fix this closure issue.  
> 
> We might just want to live with this until we get the compiler fixed. Or
> is there a different workaround like this:
> 
>     let offset = 0;
>     let offset_pr = offset;
>     pr_err!("{}", offset_pr);
>     io.write32(base, offset);

I suggested to Joel during Kangrejos that just

	pr_err!("{}", {offset});

should work, as it would create a new copy.

Best,
Gary

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ