linux-kernel - Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice implementation

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: <Z7-0pOmWO6r_KeQI@boqun-archlinux>
Date: Wed, 26 Feb 2025 16:41:08 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>,
	Joel Fernandes <joelagnelf@...dia.com>,
	Alexandre Courbot <acourbot@...dia.com>,
	Dave Airlie <airlied@...il.com>, Gary Guo <gary@...yguo.net>,
	Joel Fernandes <joel@...lfernandes.org>,
	John Hubbard <jhubbard@...dia.com>, Ben Skeggs <bskeggs@...dia.com>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	nouveau@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
	paulmck@...nel.org
Subject: Re: [RFC PATCH 0/3] gpu: nova-core: add basic timer subdevice
 implementation

On Wed, Feb 26, 2025 at 07:47:30PM -0400, Jason Gunthorpe wrote:
[...]
> 
> > Other abstractions do consider this though, e.g. the upcoming hrtimer work. [1]
> 
> Does it??? hrtimer uses function pointers. Any time you take a
> function pointer you have to reason about how does the .text lifetime
> work relative to the usage of the function pointer.
> 
> So how does [1] guarentee that the hrtimer C code will not call the
> function pointer after driver remove() completes?
> 
> My rust is aweful, but it looks to me like the timer lifetime is
> linked to the HrTimerHandle lifetime, but nothing seems to hard link
> that to the driver bound, or module lifetime?
> 

So to write a module, normally you need to have a module struct, e.g.

	struct MyModule { ... }

and if a hrtimer is used by MyModule, you can put an HrTimerHandle in
it:

	struct MyModule {
	    ...
	    handle: Option<HrTimerHandle>
	}

, when module unloaded, every field of MyModule will call their drop()
function, and HrTimerHandle's drop() will cancel the hrtimer, so that
the function pointer won't be referenced by hrtimer core.

And if you don't store the HrTimerHandle anywhere, like you drop() it
right after start a hrtimer, it will immediately stop the timer. Does
this make sense?

Regards,
Boqun

> This is what I'm talking about, the design pattern you are trying to
> fix with revocable is *everywhere* in the C APIs, it is very subtle,
> but must be considered. One solution would be to force hrtimer into
> a revocable too.
> 
> And on and on for basically every kernel API that uses function
> pointers.
> 
> This does not seem reasonable to me at all, it certainly isn't better
> than the standard pattern.
> 
> > be) also reflected by the corresponding abstraction. Dropping a
> > MiscDeviceRegistration [2] on module_exit() for instance will ensure that there
> > are no concurrent IOCTLs, just like the corresponding C code.
> 
> The way misc device works you can't unload the module until all the
> FDs are closed and the misc code directly handles races with opening
> new FDs while modules are unloading. It is quite a different scheme
> than discussed in this thread.
> 
> Jason