Re: [PATCH 07/23] watchdog: Sync up with runtime change of isolated CPUs

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Silent overwrite and clobbering of userspace `watchdog_cpumask` sysctl configuration.
--

commit c51e1c592377490b0da49dd889417304c34badc2
Author: Waiman Long <longman@redhat.com>

watchdog: Sync up with runtime change of isolated CPUs

This commit updates the watchdog to exclude isolated CPUs at runtime by
adding a new watchdog_cpumask_update() helper. It modifies watchdog_cpumask
and watchdog_allowed_mask when the HK_TYPE_KERNEL_NOISE cpumask is updated.

> diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> index 7d675781bc917..f35374abf457c 100644
> --- a/kernel/watchdog.c
> +++ b/kernel/watchdog.c
[ ... ]
> +/**
> + * watchdog_cpumask_update - update watchdog_cpumask & watchdog_allowed_mask
> + * @isol_mask: cpumask of isolated CPUs
> + *
> + * Update watchdog_cpumask and watchdog_allowed_mask to be inverse of the
> + * given isolated cpumask to disable watchdog activities on isolated CPUs.
> + * It should be called with the affected CPUs in offline state which will be
> + * brought up online later.
> + *
> + * Any changes made in watchdog_cpumask by users via the sysctl parameter will
> + * be overridden. However, proc_watchdog_update() isn't called. So change will
> + * only happens on CPUs that will brought up later on to minimize changes to
> + * the existing watchdog configuration.
> + */
> +void watchdog_cpumask_update(struct cpumask *isol_mask)
> +{
> +	mutex_lock(&watchdog_mutex);
> +	cpumask_andnot(&watchdog_cpumask, cpu_possible_mask, isol_mask);

Does this unintentionally discard the existing sysctl configuration for
kernel.watchdog_cpumask?

By using cpu_possible_mask as the base, it looks like this unconditionally
overwrites the entire watchdog_cpumask with all possible CPUs except the
newly isolated ones. 

If an administrator had explicitly disabled watchdogs on CPUs 1-6 using the
sysctl interface, and later dynamically isolates CPU 7, would this operation
silently clobber the user API contract and re-enable watchdogs on CPUs 1-6
when they are next onlined or reconfigured?

> +#ifdef CONFIG_SOFTLOCKUP_DETECTOR
> +	cpumask_copy(&watchdog_allowed_mask, &watchdog_cpumask);
> +#endif
> +	mutex_unlock(&watchdog_mutex);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=7