Re: [PATCH] Drivers: hv: Avoid infinite retry loop in init_vp_index()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch inadvertently leaves `check_hkcpu` as `false` for non-performance VMbus channels, unintentionally disabling `HK_TYPE_MANAGED_IRQ` isolation checks and allowing their interrupts to be assigned to isolated CPUs.
- [High] The fix actively violates the user's CPU isolation configuration by forcing interrupts onto isolated CPUs instead of simply skipping incompatible NUMA nodes.
--

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

Drivers: hv: Avoid infinite retry loop in init_vp_index()

This commit addresses an infinite retry loop in init_vp_index() by allowing
the housekeeping cpumask to be ignored if the allocated cpumask has already
been cleared. It introduces a check_hkcpu flag in struct vmbus_channel to
control whether the housekeeping CPU check should be performed.

Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org [1]

> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index a044fd3b3c4e7..30d369ee4bf4a 100644
> --- a/drivers/hv/channel_mgmt.c
> +++ b/drivers/hv/channel_mgmt.c
> @@ -774,6 +774,7 @@ static void init_vp_index(struct vmbus_channel *channel)
>  	}
>  
>  	for (i = 1; i <= ncpu + 1; i++) {
> +		channel->check_hkcpu = true;

[Severity: High]
Since struct vmbus_channel is allocated with kzalloc(), check_hkcpu starts
as false. In init_vp_index(), non-performance channels return early before
this loop:

    if (!perf_chn || ...) {
        channel->target_cpu = VMBUS_CONNECT_CPU;
        return;
    }

Does this unintentionally leave check_hkcpu disabled permanently for
non-performance channels?

If CPU hotplug (hv_synic_cleanup() -> hv_pick_new_cpu()) or sysfs
target_cpu_store() later changes the CPU for a non-performance channel, it
appears vmbus_channel_set_cpu() will skip the housekeeping check because
check_hkcpu is false.

Could this allow non-performance VMbus channels to be assigned to isolated
CPUs, bypassing CPU isolation protections?

>  		while (true) {
>  			numa_node = next_numa_node_id++;
>  			if (numa_node == nr_node_ids) {
> @@ -788,14 +789,21 @@ static void init_vp_index(struct vmbus_channel *channel)
>  
>  retry:
>  		cpumask_xor(available_mask, allocated_mask, cpumask_of_node(numa_node));
> -		cpumask_and(available_mask, available_mask, hk_mask);
> +		if (channel->check_hkcpu)
> +			cpumask_and(available_mask, available_mask, hk_mask);
>  
>  		if (cpumask_empty(available_mask)) {
>  			/*
>  			 * We have cycled through all the CPUs in the node;
> -			 * reset the allocated map.
> +			 * reset the allocated map. If the allocated map has
> +			 * already been cleared, we will have to ignore the
> +			 * HK_TYPE_MANAGED_IRQ housekeeping cpumask as its use
> +			 * is on a best effort basis, not a must.
>  			 */
> -			cpumask_clear(allocated_mask);
> +			if (!cpumask_empty(allocated_mask))
> +				cpumask_clear(allocated_mask);
> +			else
> +				channel->check_hkcpu = false;
>  			goto retry;
>  		}

[Severity: High]
Does this logic actively violate CPU isolation configurations by forcing
interrupts onto isolated CPUs?

When the allocator selects a NUMA node where all CPUs are isolated (none are
in hk_mask), clearing check_hkcpu forces the interrupt onto one of those
isolated CPUs.

Since cpumask_empty(hk_mask) is checked globally before the loop, there is
guaranteed to be at least one housekeeping CPU on some other node in the
system.

Would it be better to simply skip incompatible nodes entirely instead
of permanently overriding the housekeeping mask and forcing high-throughput
performance channels onto isolated CPUs?

[ ... ]

> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 5ebdbe24b5a1e..4cb9da8467f29 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1745,7 +1745,8 @@ int vmbus_channel_set_cpu(struct vmbus_channel *channel, u32 target_cpu)
>  	if (target_cpu >= nr_cpumask_bits)
>  		return -EINVAL;
>  
> -	if (!cpumask_test_cpu(target_cpu, housekeeping_cpumask(HK_TYPE_MANAGED_IRQ)))
> +	if (channel->check_hkcpu &&
> +	    !cpumask_test_cpu(target_cpu, housekeeping_cpumask(HK_TYPE_MANAGED_IRQ)))
>  		return -EINVAL;
>  
>  	if (!cpu_online(target_cpu))

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