linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/kexec: Fix the return of uninitialized variable
@ 2024-09-30  7:56 Zhang Zekun
  2024-09-30  8:27 ` Christophe Leroy
  2024-11-17 12:09 ` Michael Ellerman
  0 siblings, 2 replies; 5+ messages in thread
From: Zhang Zekun @ 2024-09-30  7:56 UTC (permalink / raw)
  To: mpe, npiggin, christophe.leroy, naveen, maddy, linuxppc-dev
  Cc: chenjun102, zhangzekun11

The of_property_read_u64() can fail and remain the variable uninitialized,
which will then be used. Return error if we failed to read the property.

Fixes: 2e6bd221d96f ("powerpc/kexec_file: Enable early kernel OPAL calls")
Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
---
 arch/powerpc/kexec/file_load_64.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 9738adabeb1f..dc65c1391157 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -736,13 +736,18 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
 	if (dn) {
 		u64 val;
 
-		of_property_read_u64(dn, "opal-base-address", &val);
+		ret = of_property_read_u64(dn, "opal-base-address", &val);
+		if (ret)
+			goto out;
+
 		ret = kexec_purgatory_get_set_symbol(image, "opal_base", &val,
 						     sizeof(val), false);
 		if (ret)
 			goto out;
 
-		of_property_read_u64(dn, "opal-entry-address", &val);
+		ret = of_property_read_u64(dn, "opal-entry-address", &val);
+		if (ret)
+			goto out;
 		ret = kexec_purgatory_get_set_symbol(image, "opal_entry", &val,
 						     sizeof(val), false);
 	}
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Fix the return of uninitialized variable
  2024-09-30  7:56 [PATCH] powerpc/kexec: Fix the return of uninitialized variable Zhang Zekun
@ 2024-09-30  8:27 ` Christophe Leroy
  2024-09-30  9:01   ` Madhavan Srinivasan
  2024-11-17 12:09 ` Michael Ellerman
  1 sibling, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2024-09-30  8:27 UTC (permalink / raw)
  To: Zhang Zekun, mpe, npiggin, naveen, maddy, linuxppc-dev; +Cc: chenjun102



Le 30/09/2024 à 09:56, Zhang Zekun a écrit :
> [Vous ne recevez pas souvent de courriers de zhangzekun11@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> The of_property_read_u64() can fail and remain the variable uninitialized,

Replace "remain" by "leave".

> which will then be used. Return error if we failed to read the property.

Rewrite to avoid "we".  For instance "Return error if reading the 
property failed"

> 
> Fixes: 2e6bd221d96f ("powerpc/kexec_file: Enable early kernel OPAL calls")
> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
> ---
>   arch/powerpc/kexec/file_load_64.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index 9738adabeb1f..dc65c1391157 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -736,13 +736,18 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>          if (dn) {
>                  u64 val;
> 
> -               of_property_read_u64(dn, "opal-base-address", &val);
> +               ret = of_property_read_u64(dn, "opal-base-address", &val);
> +               if (ret)
> +                       goto out;
> +
>                  ret = kexec_purgatory_get_set_symbol(image, "opal_base", &val,
>                                                       sizeof(val), false);
>                  if (ret)
>                          goto out;
> 
> -               of_property_read_u64(dn, "opal-entry-address", &val);
> +               ret = of_property_read_u64(dn, "opal-entry-address", &val);
> +               if (ret)
> +                       goto out;
>                  ret = kexec_purgatory_get_set_symbol(image, "opal_entry", &val,
>                                                       sizeof(val), false);
>          }
> --
> 2.17.1
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Fix the return of uninitialized variable
  2024-09-30  8:27 ` Christophe Leroy
@ 2024-09-30  9:01   ` Madhavan Srinivasan
  2024-10-08  8:53     ` zhangzekun (A)
  0 siblings, 1 reply; 5+ messages in thread
From: Madhavan Srinivasan @ 2024-09-30  9:01 UTC (permalink / raw)
  To: Christophe Leroy, Zhang Zekun, mpe, npiggin, naveen, linuxppc-dev
  Cc: chenjun102



On 9/30/24 1:57 PM, Christophe Leroy wrote:
> 
> 
> Le 30/09/2024 à 09:56, Zhang Zekun a écrit :
>> [Vous ne recevez pas souvent de courriers de zhangzekun11@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>>
>> The of_property_read_u64() can fail and remain the variable uninitialized,
> 
> Replace "remain" by "leave".
> 
>> which will then be used. Return error if we failed to read the property.
> 
> Rewrite to avoid "we".  For instance "Return error if reading the property failed"
> 
>>
>> Fixes: 2e6bd221d96f ("powerpc/kexec_file: Enable early kernel OPAL calls")
>> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
>> ---
>>   arch/powerpc/kexec/file_load_64.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>> index 9738adabeb1f..dc65c1391157 100644
>> --- a/arch/powerpc/kexec/file_load_64.c
>> +++ b/arch/powerpc/kexec/file_load_64.c
>> @@ -736,13 +736,18 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>>          if (dn) {
>>                  u64 val;

Instead cant we init val as -1 ??
Why to add check?


>>
>> -               of_property_read_u64(dn, "opal-base-address", &val);
>> +               ret = of_property_read_u64(dn, "opal-base-address", &val);
>> +               if (ret)
>> +                       goto out;
>> +
>>                  ret = kexec_purgatory_get_set_symbol(image, "opal_base", &val,
>>                                                       sizeof(val), false);
>>                  if (ret)
>>                          goto out;
>>
>> -               of_property_read_u64(dn, "opal-entry-address", &val);
>> +               ret = of_property_read_u64(dn, "opal-entry-address", &val);
>> +               if (ret)
>> +                       goto out;
>>                  ret = kexec_purgatory_get_set_symbol(image, "opal_entry", &val,
>>                                                       sizeof(val), false);
>>          }
>> -- 
>> 2.17.1
>>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Fix the return of uninitialized variable
  2024-09-30  9:01   ` Madhavan Srinivasan
@ 2024-10-08  8:53     ` zhangzekun (A)
  0 siblings, 0 replies; 5+ messages in thread
From: zhangzekun (A) @ 2024-10-08  8:53 UTC (permalink / raw)
  To: Madhavan Srinivasan
  Cc: chenjun102, Christophe Leroy, mpe, npiggin, naveen, linuxppc-dev

在 2024/9/30 17:01, Madhavan Srinivasan 写道:
> 
> 
> On 9/30/24 1:57 PM, Christophe Leroy wrote:
>>
>>
>> Le 30/09/2024 à 09:56, Zhang Zekun a écrit :
>>> [Vous ne recevez pas souvent de courriers de zhangzekun11@huawei.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> The of_property_read_u64() can fail and remain the variable uninitialized,
>>
>> Replace "remain" by "leave".
>>
>>> which will then be used. Return error if we failed to read the property.
>>
>> Rewrite to avoid "we".  For instance "Return error if reading the property failed"
>>
>>>
>>> Fixes: 2e6bd221d96f ("powerpc/kexec_file: Enable early kernel OPAL calls")
>>> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>
>>> ---
>>>    arch/powerpc/kexec/file_load_64.c | 9 +++++++--
>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
>>> index 9738adabeb1f..dc65c1391157 100644
>>> --- a/arch/powerpc/kexec/file_load_64.c
>>> +++ b/arch/powerpc/kexec/file_load_64.c
>>> @@ -736,13 +736,18 @@ int setup_purgatory_ppc64(struct kimage *image, const void *slave_code,
>>>           if (dn) {
>>>                   u64 val;
> 
> Instead cant we init val as -1 ??
> Why to add check?
> 

Hi, Madhavan,

I am not sure when the default value -1 will be checked, and it would be 
more obvious to me to add check when reading property failed. Besides, 
in arch/powerpc, checking the return val when of_property_read_u64() 
failed seems to be a more common way.

Best Regards,
Zekun


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] powerpc/kexec: Fix the return of uninitialized variable
  2024-09-30  7:56 [PATCH] powerpc/kexec: Fix the return of uninitialized variable Zhang Zekun
  2024-09-30  8:27 ` Christophe Leroy
@ 2024-11-17 12:09 ` Michael Ellerman
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Ellerman @ 2024-11-17 12:09 UTC (permalink / raw)
  To: mpe, npiggin, christophe.leroy, naveen, maddy, linuxppc-dev,
	Zhang Zekun
  Cc: chenjun102

On Mon, 30 Sep 2024 15:56:28 +0800, Zhang Zekun wrote:
> The of_property_read_u64() can fail and remain the variable uninitialized,
> which will then be used. Return error if we failed to read the property.
> 
> 

Applied to powerpc/next.

[1/1] powerpc/kexec: Fix the return of uninitialized variable
      https://git.kernel.org/powerpc/c/83b5a407fbb73e6965adfb4bd0a803724bf87f96

cheers


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-11-17 12:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30  7:56 [PATCH] powerpc/kexec: Fix the return of uninitialized variable Zhang Zekun
2024-09-30  8:27 ` Christophe Leroy
2024-09-30  9:01   ` Madhavan Srinivasan
2024-10-08  8:53     ` zhangzekun (A)
2024-11-17 12:09 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).