netdev - Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY drivers

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]

Date: Fri, 17 Nov 2023 15:28:46 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Andrew Lunn <andrew@...n.ch>, Alice Ryhl <aliceryhl@...gle.com>,
	fujita.tomonori@...il.com, benno.lossin@...ton.me,
	miguel.ojeda.sandonis@...il.com, netdev@...r.kernel.org,
	rust-for-linux@...r.kernel.org, tmgross@...ch.edu,
	wedsonaf@...il.com
Subject: Re: [PATCH net-next v7 1/5] rust: core abstractions for network PHY
 drivers

On Fri, Nov 17, 2023 at 02:50:45PM -0500, Greg KH wrote:
> On Fri, Nov 17, 2023 at 02:53:44PM +0100, Andrew Lunn wrote:
> > > I would change this to "it's okay to call phy_drivers_unregister from a
> > > different thread than the one in which phy_drivers_register was called".
> > 
> > This got me thinking about 'threads'. For register and unregister, we

Just to make things clear for discussion, the "thread" here (when we are
talking about trait `Send` and `Sync`) means "contexts" (thread/process
contexts, irq contexts, etc).

When we say a type is `Send`, it means the object of that type can be
created in one context, passed to another context (could be the same
type of context, e.g. sending between two thread/process contexts) and
dropped there. For example, if you have a work_struct embedded type, you
can create it in the driver code and pass it to workqueue, and when the
work is done, you can free it in the workqueue context.

One example of not `Send` type (or `!Send`) is spinlock guard:

	let guard: Guard<..> = some_lock.lock();

creating a Guard means "spin_lock()" and dropping a Guard means
"spin_unlock()", since we cannot acquire a spinlock in one context and
release it in another context in kernel, so `Guard<..>` is `!Send`.

Back to the code here:

	unsafe impl Send for Registration {}

the safety comment needs to explain why the `Registration::drop` is safe
to call in a different context.

Hope this helps.

Regards,
Boqun

> > are talking abut the kernel modules module_init() and module_exit()
> > function. module_init() can be called before user space is even
> > started, but it could also be called by insmod. module_exit() could be
> > called by rmmod, but it could also be the kernel, after user space has
> > gone away on shutdown.
> 
> The kernel will not call module_exit() on shutdown.  Or has something
> changed recently?
> 
> > We are always in a context which can block, but
> > i never really think of this being threads. You can start a kernel
> > thread, and have some data structure exclusively used by that kernel
> > thread, but that is pretty unusual.
> > 
> > So i would probably turn this commenting around. Only comment like
> > this in the special case that a kernel thread exists, and it is
> > expected to have exclusive access.
> 
> With the driver model, you can be sure that your probe/release functions
> for the bus will never be called at the same time, and module_init/exit
> can never be called at the same time, so perhaps this isn't an issue?
> 
> thanks,
> 
> greg k-h
>