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

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

[Waiman Long]

There is a retry loop in init_vp_index() where the CPUs from a certain
node are stripped out if they have already been in the allocated cpumask
or not in HK_TYPE_MANAGED_IRQ housekeeping cpumask. If there is no
CPU left, the allocated cpumask is ignored and the process is retried
again. However, if the HK_TYPE_MANAGED_IRQ housekeeping cpumask turns
out not to contain any CPU in that particular node, that will become an
infinite retry loop.  This particular problem was reported by sashiko
[1]. This should rarely happen, but we still need to guard against this.

Fix this infinite loop problem by ignoring the HK_TYPE_MANAGED_IRQ
housekeeping cpumask if the allocated cpumask has already been
cleared before. Since the HK_TYPE_MANAGED_IRQ housekeeping cpumask
is supposed to be used on a best effort basis, it is OK to ignore it
in this particular case. Also add a check_hkcpu boolean flag in struct
vmbus_channel to control the HK_TYPE_MANAGED_IRQ housekeeping CPU check
in target_cpu_store() and init_vp_index().

Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org [1]
Fixes: 6640b5df1a38 ("Drivers: hv: vmbus: Don't assign VMbus channel interrupts to isolated CPUs")
Signed-off-by: Waiman Long <longman@redhat.com>
---
 drivers/hv/channel_mgmt.c | 14 +++++++++++---
 drivers/hv/vmbus_drv.c    |  3 ++-
 include/linux/hyperv.h    |  7 +++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 89d214dda360..30d91668e1c5 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;
 		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;
 		}
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 6824bd7cb3c4..bea578cd0aa7 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1751,7 +1751,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))
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index a2b484679eb4..0d8df79c26ca 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -831,6 +831,13 @@ struct vmbus_channel {
 	 */
 	bool out_full_flag;
 
+	/*
+	 * Check target_cpu in target_cpu_store() to make sure that it is in the
+	 * HK_TYPE_MANAGED_IRQ housekeeping cpumask and reject it if not when
+	 * the flag is set.
+	 */
+	bool check_hkcpu;
+
 	/* Channel callback's invoked in softirq context */
 	struct tasklet_struct callback_event;
 	void (*onchannel_callback)(void *context);
-- 
2.55.0

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

[sashiko-bot]

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

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

[Waiman Long]

On 8/26/26 1:37 PM, Waiman Long wrote:
> There is a retry loop in init_vp_index() where the CPUs from a certain
> node are stripped out if they have already been in the allocated cpumask
> or not in HK_TYPE_MANAGED_IRQ housekeeping cpumask. If there is no
> CPU left, the allocated cpumask is ignored and the process is retried
> again. However, if the HK_TYPE_MANAGED_IRQ housekeeping cpumask turns
> out not to contain any CPU in that particular node, that will become an
> infinite retry loop.  This particular problem was reported by sashiko
> [1]. This should rarely happen, but we still need to guard against this.
>
> Fix this infinite loop problem by ignoring the HK_TYPE_MANAGED_IRQ
> housekeeping cpumask if the allocated cpumask has already been
> cleared before. Since the HK_TYPE_MANAGED_IRQ housekeeping cpumask
> is supposed to be used on a best effort basis, it is OK to ignore it
> in this particular case. Also add a check_hkcpu boolean flag in struct
> vmbus_channel to control the HK_TYPE_MANAGED_IRQ housekeeping CPU check
> in target_cpu_store() and init_vp_index().
>
> Link: https://sashiko.dev/#/message/20260422030903.E1BFCC2BCB0%40smtp.kernel.org [1]
> Fixes: 6640b5df1a38 ("Drivers: hv: vmbus: Don't assign VMbus channel interrupts to isolated CPUs")
> Signed-off-by: Waiman Long <longman@redhat.com>

Please ignore this patch. I have sent out a v2 after reviewing feedback 
from sashiko.

Cheers,
Longman

> ---
>   drivers/hv/channel_mgmt.c | 14 +++++++++++---
>   drivers/hv/vmbus_drv.c    |  3 ++-
>   include/linux/hyperv.h    |  7 +++++++
>   3 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
> index 89d214dda360..30d91668e1c5 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;
>   		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;
>   		}
>   
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 6824bd7cb3c4..bea578cd0aa7 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1751,7 +1751,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))
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index a2b484679eb4..0d8df79c26ca 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -831,6 +831,13 @@ struct vmbus_channel {
>   	 */
>   	bool out_full_flag;
>   
> +	/*
> +	 * Check target_cpu in target_cpu_store() to make sure that it is in the
> +	 * HK_TYPE_MANAGED_IRQ housekeeping cpumask and reject it if not when
> +	 * the flag is set.
> +	 */
> +	bool check_hkcpu;
> +
>   	/* Channel callback's invoked in softirq context */
>   	struct tasklet_struct callback_event;
>   	void (*onchannel_callback)(void *context);