netdev - Re: iopoll 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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f7906232-c2c7-4fd6-be6b-7e96bbfbbcad@lunn.ch>
Date: Thu, 3 Oct 2024 18:00:36 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Boqun Feng <boqun.feng@...il.com>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, dirk.behme@...bosch.com,
	aliceryhl@...gle.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
Subject: Re: iopoll abstraction

> > fn read_poll_timeout<Op, Cond, T: Copy>(
> >     mut op: Op,
> >     cond: Cond,
> >     sleep: Delta,
> >     timeout: Delta,
> > ) -> Result<T>
> > where
> >     Op: FnMut() -> Result<T>,
> >     Cond: Fn(T) -> bool,
> > {
> >     let timeout = Ktime::ktime_get() + timeout;
> >     let ret = loop {
> >         let val = op()?;
> >         if cond(val) {
> >             break Ok(val);
> >         }
> >         kernel::delay::sleep(sleep);
> > 
> >         if Ktime::ktime_get() > timeout {
> >             break Err(code::ETIMEDOUT);
> >         }
> >     };
> > 
> >     ret
> > }

This appears to have the usual bug when people implement it themselves
and i then point them at iopoll.h, which so far as been bug free.

kernel::delay::sleep(sleep) can sleep for an arbitrary amount of time
greater than sleep, if the system is busy doing other things. You
might only get around this loop once, and exit with ETIMEOUT, but
while you have been sleeping a long time the hardware has completed
its operation, but you never check.

There must be a call to cond() after the timeout to handle this
condition.

And this is not theoretical. I had a very reproducible case of this
during the boot of a device. It is less likely today, with SMP
systems, and all the RT patches, but if it does happen, it will be
very hard to track down.

	Andrew