Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1407171 > unrolled thread

[PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data

Started byHoan Tran <hotran@apm.com>
First post2016-05-25 21:20 +0200
Last post2016-05-27 18:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data Hoan Tran <hotran@apm.com> - 2016-05-25 21:20 +0200
    Re: [PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data Ashwin Chaugule <ashwin.chaugule@linaro.org> - 2016-05-27 18:20 +0200
      Re: [PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data Hoan Tran <hotran@apm.com> - 2016-05-27 18:50 +0200

#1407171 — [PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data

FromHoan Tran <hotran@apm.com>
Date2016-05-25 21:20 +0200
Subject[PATCH] acpi: cppc: Prevent cpc_desc_ptr points to the invalid data
Message-ID<rCMvv-4LK-5@gated-at.bofh.it>
When CPPC fails to request PCC channel, the CPC data is freed
and cpc_desc_ptr points to the invalid data. This change prevents
this issue by moving cpc_desc_ptr assignment after PCC channel
request.

Signed-off-by: Hoan Tran <hotran@apm.com>
---
 drivers/acpi/cppc_acpi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 8adac69..85fd8f7 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -595,9 +595,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	/* Store CPU Logical ID */
 	cpc_ptr->cpu_id = pr->id;
 
-	/* Plug it into this CPUs CPC descriptor. */
-	per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
-
 	/* Parse PSD data for this CPU */
 	ret = acpi_get_psd(cpc_ptr, handle);
 	if (ret)
@@ -610,6 +607,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			goto out_free;
 	}
 
+	/* Plug PSD data into this CPUs CPC descriptor. */
+	per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
+
 	/* Everything looks okay */
 	pr_debug("Parsed CPC struct for CPU: %d\n", pr->id);
 
-- 
1.9.1

[toc] | [next] | [standalone]


#1408167

FromAshwin Chaugule <ashwin.chaugule@linaro.org>
Date2016-05-27 18:20 +0200
Message-ID<rDsEq-5Ht-29@gated-at.bofh.it>
In reply to#1407171
On 25 May 2016 at 15:09, Hoan Tran <hotran@apm.com> wrote:
> When CPPC fails to request PCC channel, the CPC data is freed
> and cpc_desc_ptr points to the invalid data. This change prevents
> this issue by moving cpc_desc_ptr assignment after PCC channel
> request.
>
> Signed-off-by: Hoan Tran <hotran@apm.com>
> ---
>  drivers/acpi/cppc_acpi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 8adac69..85fd8f7 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -595,9 +595,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>         /* Store CPU Logical ID */
>         cpc_ptr->cpu_id = pr->id;
>
> -       /* Plug it into this CPUs CPC descriptor. */
> -       per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
> -
>         /* Parse PSD data for this CPU */
>         ret = acpi_get_psd(cpc_ptr, handle);
>         if (ret)
> @@ -610,6 +607,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>                         goto out_free;
>         }
>
> +       /* Plug PSD data into this CPUs CPC descriptor. */
> +       per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
> +

Are you seeing a real problem without this change? I'm missing where
this pointer is dereferenced if the PCC channel request fails.

Thanks,
Ashwin.

[toc] | [prev] | [next] | [standalone]


#1408182

FromHoan Tran <hotran@apm.com>
Date2016-05-27 18:50 +0200
Message-ID<rDt7s-5Rp-9@gated-at.bofh.it>
In reply to#1408167
Hi Ashwin,

Yes, I saw kernel crash.
As cpc_desc_ptr is not NULL, cppc_cpufreq_init() still can pass then
crash during cppc_get_perf_caps() access CPPC shared memory.

It's not only "PCC channel request fail" can create this issue but
"acpi_get_psd() fail" also creates it

Thanks
Hoan

On Fri, May 27, 2016 at 9:10 AM, Ashwin Chaugule
<ashwin.chaugule@linaro.org> wrote:
> On 25 May 2016 at 15:09, Hoan Tran <hotran@apm.com> wrote:
>> When CPPC fails to request PCC channel, the CPC data is freed
>> and cpc_desc_ptr points to the invalid data. This change prevents
>> this issue by moving cpc_desc_ptr assignment after PCC channel
>> request.
>>
>> Signed-off-by: Hoan Tran <hotran@apm.com>
>> ---
>>  drivers/acpi/cppc_acpi.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
>> index 8adac69..85fd8f7 100644
>> --- a/drivers/acpi/cppc_acpi.c
>> +++ b/drivers/acpi/cppc_acpi.c
>> @@ -595,9 +595,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>>         /* Store CPU Logical ID */
>>         cpc_ptr->cpu_id = pr->id;
>>
>> -       /* Plug it into this CPUs CPC descriptor. */
>> -       per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
>> -
>>         /* Parse PSD data for this CPU */
>>         ret = acpi_get_psd(cpc_ptr, handle);
>>         if (ret)
>> @@ -610,6 +607,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
>>                         goto out_free;
>>         }
>>
>> +       /* Plug PSD data into this CPUs CPC descriptor. */
>> +       per_cpu(cpc_desc_ptr, pr->id) = cpc_ptr;
>> +
>
> Are you seeing a real problem without this change? I'm missing where
> this pointer is dereferenced if the PCC channel request fails.
>
> Thanks,
> Ashwin.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web