Re: [PATCH] powerpc/hv-gpci: fix preempt count leak in sysfs show paths

[Shrikanth Hegde <sshegde@linux.ibm.com>]

On 5/8/26 9:42 AM, Aboorva Devarajan wrote:
> Four sysfs show() callbacks in hv-gpci take get_cpu_var(hv_gpci_reqb)
> (which calls preempt_disable()) but only call the matching put_cpu_var()
> on the error path under the 'out:' label. Every successful read leaks
> one preempt_disable():
> 
>    processor_bus_topology_show()
>    processor_config_show()
>    affinity_domain_via_virtual_processor_show()
>    affinity_domain_via_domain_show()
> 
> (affinity_domain_via_partition_show() was already correct.)
> 
> On a CONFIG_PREEMPT=y kernel, repeated reads raise preempt_count and
> eventually return to userspace with preemption still disabled. The
> next user-mode page fault then hits faulthandler_disabled() == 1,
> gets forced to SIGSEGV, and the resulting coredump trips
> 'BUG: scheduling while atomic' in call_usermodehelper_exec ->
> wait_for_completion_state -> schedule:
> 
>    BUG: scheduling while atomic: <task>/<pid>/0x00000004
>    ...
>    __schedule_bug+0x6c/0x90
>    __schedule+0x58c/0x13a0
>    schedule+0x48/0x1a0
>    schedule_timeout+0x104/0x170
>    wait_for_completion_state+0x16c/0x330
>    call_usermodehelper_exec+0x254/0x2d0
>    vfs_coredump+0x1050/0x2590
>    get_signal+0xb9c/0xc80
>    do_notify_resume+0xf8/0x470
> 
> Add an out_success label that calls put_cpu_var() before returning
> the byte count, mirroring affinity_domain_via_partition_show().
> 
> Fixes: 71f1c39647d8 ("powerpc/hv_gpci: Add sysfs file inside hv_gpci device to show processor bus topology information")
> Fixes: 1a160c2a13c6 ("powerpc/hv_gpci: Add sysfs file inside hv_gpci device to show processor config information")
> Fixes: 71a7ccb478fc ("powerpc/hv_gpci: Add sysfs file inside hv_gpci device to show affinity domain via virtual processor information")
> Fixes: a69a57cac1ec ("powerpc/hv_gpci: Add sysfs file inside hv_gpci device to show affinity domain via domain information")
> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> ---
>   arch/powerpc/perf/hv-gpci.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c
> index 5cac2cf3bd1e5..10c82cf8f5b39 100644
> --- a/arch/powerpc/perf/hv-gpci.c
> +++ b/arch/powerpc/perf/hv-gpci.c
> @@ -210,7 +210,7 @@ static ssize_t processor_bus_topology_show(struct device *dev, struct device_att
>   			0, 0, buf, &n, arg);
>   
>   	if (!ret)
> -		return n;
> +		goto out_success;
>   
>   	if (ret != H_PARAMETER)
>   		goto out;
> @@ -244,12 +244,14 @@ static ssize_t processor_bus_topology_show(struct device *dev, struct device_att
>   				starting_index, 0, buf, &n, arg);
>   
>   		if (!ret)
> -			return n;
> +			goto out_success;
>   
>   		if (ret != H_PARAMETER)
>   			goto out;
>   	}
>   
> +out_success:
> +	put_cpu_var(hv_gpci_reqb);
>   	return n;
>   
>   out:
> @@ -278,7 +280,7 @@ static ssize_t processor_config_show(struct device *dev, struct device_attribute
>   			0, 0, buf, &n, arg);
>   
>   	if (!ret)
> -		return n;
> +		goto out_success;
>   
>   	if (ret != H_PARAMETER)
>   		goto out;
> @@ -312,12 +314,14 @@ static ssize_t processor_config_show(struct device *dev, struct device_attribute
>   				starting_index, 0, buf, &n, arg);
>   
>   		if (!ret)
> -			return n;
> +			goto out_success;
>   
>   		if (ret != H_PARAMETER)
>   			goto out;
>   	}
>   
> +out_success:
> +	put_cpu_var(hv_gpci_reqb);
>   	return n;
>   
>   out:
> @@ -346,7 +350,7 @@ static ssize_t affinity_domain_via_virtual_processor_show(struct device *dev,
>   			0, 0, buf, &n, arg);
>   
>   	if (!ret)
> -		return n;
> +		goto out_success;
>   
>   	if (ret != H_PARAMETER)
>   		goto out;
> @@ -382,12 +386,14 @@ static ssize_t affinity_domain_via_virtual_processor_show(struct device *dev,
>   				starting_index, secondary_index, buf, &n, arg);
>   
>   		if (!ret)
> -			return n;
> +			goto out_success;
>   
>   		if (ret != H_PARAMETER)
>   			goto out;
>   	}
>   
> +out_success:
> +	put_cpu_var(hv_gpci_reqb);
>   	return n;
>   
>   out:
> @@ -416,7 +422,7 @@ static ssize_t affinity_domain_via_domain_show(struct device *dev, struct device
>   			0, 0, buf, &n, arg);
>   
>   	if (!ret)
> -		return n;
> +		goto out_success;
>   
>   	if (ret != H_PARAMETER)
>   		goto out;
> @@ -448,12 +454,14 @@ static ssize_t affinity_domain_via_domain_show(struct device *dev, struct device
>   					starting_index, 0, buf, &n, arg);
>   
>   		if (!ret)
> -			return n;
> +			goto out_success;
>   
>   		if (ret != H_PARAMETER)
>   			goto out;
>   	}
>   
> +out_success:
> +	put_cpu_var(hv_gpci_reqb);
>   	return n;
>   
>   out:


Thanks for fixing this. This change per se, look good to me. So,
Acked-by: Shrikanth Hegde <sshegde@linux.ibm.com>


But, it would be good to move to scope based gaurds. That would
remove all the goto's and issues like this.