Re: [PATCH] cpufreq: qcom-hw: Fix exposed stack contents

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Wed 10 Nov 07:27 PST 2021, Vladimir Zapolskiy wrote:

> On irq request it is improper to pass its designated name, which is
> allocated on stack, because the irq name string is not copied, thus
> there is a risk of leaking partial stack contents to userspace.
> 
> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>

This was already proposed and reviewed at:

https://lore.kernel.org/all/20210901084732.943248-1-ardb@kernel.org/

Could Ard's patch please be picked up by the maintainers, preferably
with above Fixes added, so we get this backported onto v5.15 stable...

> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index a2be0df7e174..b772d8ed9a77 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -375,7 +375,7 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index)
>  {
>  	struct qcom_cpufreq_data *data = policy->driver_data;
>  	struct platform_device *pdev = cpufreq_get_driver_data();
> -	char irq_name[15];
> +	char *irq_name;
>  	int ret;
>  
>  	/*
> @@ -392,9 +392,11 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index)
>  	mutex_init(&data->throttle_lock);
>  	INIT_DEFERRABLE_WORK(&data->throttle_work, qcom_lmh_dcvs_poll);
>  
> -	snprintf(irq_name, sizeof(irq_name), "dcvsh-irq-%u", policy->cpu);
> -	ret = request_threaded_irq(data->throttle_irq, NULL, qcom_lmh_dcvs_handle_irq,
> -				   IRQF_ONESHOT, irq_name, data);
> +	irq_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "dcvsh-irq-%u",

qcom_cpufreq_hw_lmh_init() might be called multiple times through the
life cycle of the associated struct device, so I find Ard's solution
preferable.

Regards,
Bjorn

> +				  policy->cpu);
> +	ret = devm_request_threaded_irq(&pdev->dev, data->throttle_irq, NULL,
> +					qcom_lmh_dcvs_handle_irq, IRQF_ONESHOT,
> +					irq_name, data);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Error registering %s: %d\n", irq_name, ret);
>  		return 0;
> -- 
> 2.32.0
>