| Message ID | 20240930075628.125138-1-zhangzekun11@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | powerpc/kexec: Fix the return of uninitialized variable | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 5 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 21 jobs. |
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 >
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 >>
在 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
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); }
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(-)