netdev - Re: [PATCH net-next v2 5/6] rust: Add read_poll

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: <e17c0b80-7518-4487-8278-f0d96fce9d8c@lunn.ch>
Date: Sun, 6 Oct 2024 16:45:21 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Boqun Feng <boqun.feng@...il.com>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
	rust-for-linux@...r.kernel.org, hkallweit1@...il.com,
	tmgross@...ch.edu, ojeda@...nel.org, alex.gaynor@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	a.hindborg@...sung.com, aliceryhl@...gle.com,
	anna-maria@...utronix.de, frederic@...nel.org, tglx@...utronix.de,
	arnd@...db.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 5/6] rust: Add read_poll_timeout function

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