Re: [PATCH v2 3/3] md/raid5: fix NULL pointer dereference in raid5_free_percpu

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Data race when checking hlist_unhashed(&conf->node) locklessly.
--

commit f000e30dfd8bae5a7346baf7add33f6d2b5316e6
Author: Cao Guanghui <[email protected]>

md/raid5: fix NULL pointer dereference in raid5_free_percpu

This commit adds a check in raid5_free_percpu() to only remove the CPU
hotplug instance if conf->node is hashed, preventing a NULL pointer
dereference on its pprev pointer during error teardown when cpuhp
registration fails.

Cc: [email protected]

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index e481c48b45093..537225c14a236 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -7548,7 +7548,8 @@ static void raid5_free_percpu(struct r5conf *conf)
>  	if (!conf->percpu)
>  		return;
>  
> -	cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
> +	if (!hlist_unhashed(&conf->node))

[Severity: Medium]
Does checking hlist_unhashed() locklessly here introduce a data race?

If one RAID array is being destroyed, it calls raid5_free_percpu() and
evaluates !hlist_unhashed(&conf->node) without holding the
cpuhp_state_mutex.

Concurrently, if another array is being created, it might take the lock
and call hlist_add_head() on the shared CPUHP_MD_RAID5_PREPARE list.
If the destroying array happens to be at the head, hlist_add_head()
updates its pprev pointer via WRITE_ONCE().

This concurrent modification can violate the kernel memory model and
trigger KCSAN load-tearing warnings. Could hlist_unhashed_lockless(),
which uses READ_ONCE(), be used here instead to safely handle concurrent
list operations?

> +		cpuhp_state_remove_instance(CPUHP_MD_RAID5_PREPARE, &conf->node);
>  	free_percpu(conf->percpu);
>  }

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