linux-kernel - Re: [PATCH 1/3] mshv: Ignore second stats page map result failure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aTCJgA3p9UKcIiU5@skinsburskii.localdomain>
Date: Wed, 3 Dec 2025 11:03:28 -0800
From: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
Cc: linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
	kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
	decui@...rosoft.com, longli@...rosoft.com, mhklinux@...look.com,
	prapal@...ux.microsoft.com, mrathor@...ux.microsoft.com,
	paekkaladevi@...ux.microsoft.com
Subject: Re: [PATCH 1/3] mshv: Ignore second stats page map result failure

On Wed, Dec 03, 2025 at 09:53:23AM -0800, Nuno Das Neves wrote:
> From: Purna Pavan Chandra Aekkaladevi <paekkaladevi@...ux.microsoft.com>
> 
> Older versions of the hypervisor do not support HV_STATS_AREA_PARENT
> and return HV_STATUS_INVALID_PARAMETER for the second stats page
> mapping request.
> 
> This results a failure in module init. Instead of failing, gracefully
> fall back to populating stats_pages[HV_STATS_AREA_PARENT] with the
> already-mapped stats_pages[HV_STATS_AREA_SELF].
> 
> Signed-off-by: Purna Pavan Chandra Aekkaladevi <paekkaladevi@...ux.microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
> ---
>  drivers/hv/mshv_root_hv_call.c | 43 ++++++++++++++++++++++++++++++----
>  drivers/hv/mshv_root_main.c    |  3 +++
>  2 files changed, 42 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hv/mshv_root_hv_call.c b/drivers/hv/mshv_root_hv_call.c
> index 598eaff4ff29..0427785bb7fe 100644
> --- a/drivers/hv/mshv_root_hv_call.c
> +++ b/drivers/hv/mshv_root_hv_call.c
> @@ -855,6 +855,24 @@ static int hv_call_map_stats_page2(enum hv_stats_object_type type,
>  	return ret;
>  }
>  
> +static int
> +hv_stats_get_area_type(enum hv_stats_object_type type,
> +		       const union hv_stats_object_identity *identity)
> +{
> +	switch (type) {
> +	case HV_STATS_OBJECT_HYPERVISOR:
> +		return identity->hv.stats_area_type;
> +	case HV_STATS_OBJECT_LOGICAL_PROCESSOR:
> +		return identity->lp.stats_area_type;
> +	case HV_STATS_OBJECT_PARTITION:
> +		return identity->partition.stats_area_type;
> +	case HV_STATS_OBJECT_VP:
> +		return identity->vp.stats_area_type;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int hv_call_map_stats_page(enum hv_stats_object_type type,
>  				  const union hv_stats_object_identity *identity,
>  				  void **addr)
> @@ -863,7 +881,7 @@ static int hv_call_map_stats_page(enum hv_stats_object_type type,
>  	struct hv_input_map_stats_page *input;
>  	struct hv_output_map_stats_page *output;
>  	u64 status, pfn;
> -	int ret = 0;
> +	int hv_status, ret = 0;
>  
>  	do {
>  		local_irq_save(flags);
> @@ -878,11 +896,28 @@ static int hv_call_map_stats_page(enum hv_stats_object_type type,
>  		pfn = output->map_location;
>  
>  		local_irq_restore(flags);
> -		if (hv_result(status) != HV_STATUS_INSUFFICIENT_MEMORY) {
> -			ret = hv_result_to_errno(status);
> +
> +		hv_status = hv_result(status);
> +		if (hv_status != HV_STATUS_INSUFFICIENT_MEMORY) {
>  			if (hv_result_success(status))
>  				break;
> -			return ret;
> +
> +			/*
> +			 * Older versions of the hypervisor do not support the
> +			 * PARENT stats area. In this case return "success" but
> +			 * set the page to NULL. The caller should check for
> +			 * this case and instead just use the SELF area.
> +			 */
> +			if (hv_stats_get_area_type(type, identity) == HV_STATS_AREA_PARENT &&
> +			    hv_status == HV_STATUS_INVALID_PARAMETER) {
> +				pr_debug_once("%s: PARENT area type is unsupported\n",
> +					      __func__);

Nit: this gebug once looks a bit odd. Why not having it printed always
(especially given the unconfidional status print a few lines below)?

Reviewed-by: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>

Thanks,
Stanislav

> +				*addr = NULL;
> +				return 0;
> +			}
> +
> +			hv_status_debug(status, "\n");
> +			return hv_result_to_errno(status);
>  		}
>  
>  		ret = hv_call_deposit_pages(NUMA_NO_NODE,
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index bc15d6f6922f..f59a4ab47685 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -905,6 +905,9 @@ static int mshv_vp_stats_map(u64 partition_id, u32 vp_index,
>  	if (err)
>  		goto unmap_self;
>  
> +	if (!stats_pages[HV_STATS_AREA_PARENT])
> +		stats_pages[HV_STATS_AREA_PARENT] = stats_pages[HV_STATS_AREA_SELF];
> +
>  	return 0;
>  
>  unmap_self:
> -- 
> 2.34.1
>