Re: [PATCH net-next v2 5/6] rust: Add read_poll_timeout function

[Andrew Lunn]

On Sat, Oct 05, 2024 at 03:22:23PM -0700, Boqun Feng wrote:
> On Sat, Oct 05, 2024 at 08:32:01PM +0200, Andrew Lunn wrote:
> > > might_sleep() is called via a wrapper so the __FILE__ and __LINE__
> > > debug info with CONFIG_DEBUG_ATOMIC_SLEEP enabled isn't what we
> > > expect; the wrapper instead of the caller.
> > 
> > So not very useful. All we know is that somewhere in Rust something is
> > sleeping in atomic context. Is it possible to do better? Does __FILE__
> > and __LINE__ exist in Rust?
> > 
> 
> Sure, you can use: 
> 
> 	https://doc.rust-lang.org/core/macro.line.html

So i guess might_sleep() needs turning into some sort of macro, calling
__might_sleep(__FILE__, __LINE__); might_resched();

> > > +    if sleep {
> > > +        // SAFETY: FFI call.
> > > +        unsafe { bindings::might_sleep() }
> > > +    }
> > 
> > What is actually unsafe about might_sleep()? It is a void foo(void)
> 
> Every extern "C" function is by default unsafe, because C doesn't have
> the concept of safe/unsafe. If you want to avoid unsafe, you could
> introduce a Rust's might_sleep() which calls into
> `bindings::might_sleep()`:
> 
> 	pub fn might_sleep() {
> 	    // SAFETY: ??
> 	    unsafe { bindings::might_sleep() }
> 	}
> 
> however, if you call a might_sleep() in a preemption disabled context
> when CONFIG_DEBUG_ATOMIC_SLEEP=n and PREEMPT=VOLUNTERY, it could means
> an unexpected RCU quiescent state, which results an early RCU grace
> period, and that may mean a use-after-free. So it's not that safe as you
> may expected.

If you call might_sleep() in a preemption disabled context you code is
already unsafe, since that is the whole point of it, to find bugs
where you use a sleeping function in atomic context. Depending on why
you are in atomic context, it might appear to work, until it does not
actually work, and bad things happen. So it is not might_sleep() which
is unsafe, it is the Rust code calling it.

	Andrew