Re: [PATCH v2 01/12] notifier: add device-managed registration APIs

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] devm_raw_notifier_chain_register structurally violates raw notifier locking contracts, causing lockless unregistration and list corruption.
- [High] devm_raw_notifier_chain_register uses GFP_KERNEL inside caller-provided locking, causing "sleeping in atomic context" panics if the lock is a spinlock.
--

--- Patch [1]: [PATCH v2 01/12] notifier: add device-managed registration APIs ---
commit c5d9a7bbfeb086f5fff3b21d687d1da2f68205b3
Author: Eliav Farber <[email protected]>

notifier: add device-managed registration APIs

Add device-managed registration functions for atomic, blocking, and raw
notifier chains. These automatically unregister the notifier when the device
is unbound, reducing driver boilerplate.

> diff --git a/kernel/notifier.c b/kernel/notifier.c
> index 2f9fe7c30287f..d8c82b237586e 100644
> --- a/kernel/notifier.c
> +++ b/kernel/notifier.c

[ ... ]

> +struct raw_notifier_chain_devres {
> +	struct raw_notifier_head *nh;
> +	struct notifier_block *nb;
> +};
> +
> +static void devm_raw_notifier_chain_unregister(struct device *dev, void *res)
> +{
> +	struct raw_notifier_chain_devres *dr = res;
> +
> +	raw_notifier_chain_unregister(dr->nh, dr->nb);

[Severity: High]
How is locking handled during device unbind? The documentation for raw notifiers
states that all locking must be provided by the caller. When the devres
framework automatically invokes devm_raw_notifier_chain_unregister() during
teardown, it calls raw_notifier_chain_unregister() completely locklessly.

Since there is no mechanism for this callback to acquire the caller's
subsystem-specific lock, will this structural flaw force concurrent,
unsynchronized list modifications and corrupt the notifier chain?

> +}
> +
> +/**
> + *	devm_raw_notifier_chain_register - Device-managed raw notifier registration
> + *	@dev: Device to tie the notifier lifetime to
> + *	@nh: Pointer to head of the raw notifier chain
> + *	@n: New entry in notifier chain
> + *
> + *	Adds a notifier to a raw notifier chain and registers a cleanup
> + *	action to automatically unregister it when @dev is unbound.
> + *	All locking must be provided by the caller.
> + *
> + *	Return:
> + *	0 on success, negative errno on error.
> + */
> +int devm_raw_notifier_chain_register(struct device *dev,
> +				     struct raw_notifier_head *nh,
> +				     struct notifier_block *n)
> +{
> +	struct raw_notifier_chain_devres *dr;
> +	int ret;
> +
> +	dr = devres_alloc(devm_raw_notifier_chain_unregister,
> +			  sizeof(*dr), GFP_KERNEL);

[Severity: High]
Will this devres_alloc() call trigger a scheduling while atomic bug? To safely
use devm_raw_notifier_chain_register() without racing, the caller must acquire
their chain-protecting lock before calling the function.

If this lock is a spinlock, which is standard for raw notifiers used in
atomic contexts, the unconditional GFP_KERNEL allocation here will sleep
while the caller holds a spinlock.

> +	if (!dr)
> +		return -ENOMEM;
> +
> +	ret = raw_notifier_chain_register(nh, n);
> +	if (ret) {
> +		devres_free(dr);
> +		return ret;
> +	}
> +
> +	dr->nh = nh;
> +	dr->nb = n;
> +	devres_add(dev, dr);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_raw_notifier_chain_register);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1