* [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-07 22:56 ` Nathan Chancellor
0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2019-03-07 22:56 UTC (permalink / raw)
To: Matthias Brugger
Cc: linux-arm-kernel, linux-mediatek, linux-kernel,
clang-built-linux, Nick Desaulniers, Nathan Chancellor
When building with -Wsometimes-uninitialized, Clang warns:
drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
used uninitialized whenever '||' condition is true
[-Werror,-Wsometimes-uninitialized]
If pwrap_write returns non-zero, pwrap_read will not be called to
initialize rdata, meaning that we will use some random uninitialized
stack value in our print statement. Zero initialize rdata in case this
happens.
Link: https://github.com/ClangBuiltLinux/linux/issues/401
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
I don't know if this is better or to just restructure the if statement
below (I'm not an expert in this code so I'll leave that up to the
maintainers to decide).
drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..2f632e8790f7 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
static int pwrap_init_cipher(struct pmic_wrapper *wrp)
{
int ret;
- u32 rdata;
+ u32 rdata = 0;
pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
--
2.21.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-07 22:56 ` Nathan Chancellor
0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2019-03-07 22:56 UTC (permalink / raw)
To: Matthias Brugger
Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
linux-mediatek, Nathan Chancellor, linux-arm-kernel
When building with -Wsometimes-uninitialized, Clang warns:
drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
used uninitialized whenever '||' condition is true
[-Werror,-Wsometimes-uninitialized]
If pwrap_write returns non-zero, pwrap_read will not be called to
initialize rdata, meaning that we will use some random uninitialized
stack value in our print statement. Zero initialize rdata in case this
happens.
Link: https://github.com/ClangBuiltLinux/linux/issues/401
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
I don't know if this is better or to just restructure the if statement
below (I'm not an expert in this code so I'll leave that up to the
maintainers to decide).
drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..2f632e8790f7 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
static int pwrap_init_cipher(struct pmic_wrapper *wrp)
{
int ret;
- u32 rdata;
+ u32 rdata = 0;
pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
--
2.21.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-07 22:56 ` Nathan Chancellor
0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2019-03-07 22:56 UTC (permalink / raw)
To: Matthias Brugger
Cc: Nick Desaulniers, linux-kernel, clang-built-linux,
linux-mediatek, Nathan Chancellor, linux-arm-kernel
When building with -Wsometimes-uninitialized, Clang warns:
drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
used uninitialized whenever '||' condition is true
[-Werror,-Wsometimes-uninitialized]
If pwrap_write returns non-zero, pwrap_read will not be called to
initialize rdata, meaning that we will use some random uninitialized
stack value in our print statement. Zero initialize rdata in case this
happens.
Link: https://github.com/ClangBuiltLinux/linux/issues/401
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
I don't know if this is better or to just restructure the if statement
below (I'm not an expert in this code so I'll leave that up to the
maintainers to decide).
drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..2f632e8790f7 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
static int pwrap_init_cipher(struct pmic_wrapper *wrp)
{
int ret;
- u32 rdata;
+ u32 rdata = 0;
pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
2019-03-07 22:56 ` Nathan Chancellor
@ 2019-03-08 0:18 ` Nick Desaulniers
-1 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-03-08 0:18 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Matthias Brugger, Linux ARM, linux-mediatek, LKML, clang-built-linux
On Thu, Mar 7, 2019 at 2:57 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> used uninitialized whenever '||' condition is true
> [-Werror,-Wsometimes-uninitialized]
>
> If pwrap_write returns non-zero, pwrap_read will not be called to
> initialize rdata, meaning that we will use some random uninitialized
> stack value in our print statement. Zero initialize rdata in case this
> happens.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/401
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> I don't know if this is better or to just restructure the if statement
> below (I'm not an expert in this code so I'll leave that up to the
> maintainers to decide).
No, I the way you have it here is most correct. That condition writes
a value somewhere, reads it back, then compares it. The write or read
could fail. Better to just initialize rdata in case the write fails.
Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8236a6c87e19..2f632e8790f7 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> - u32 rdata;
> + u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
> --
> 2.21.0
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-08 0:18 ` Nick Desaulniers
0 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2019-03-08 0:18 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Matthias Brugger, clang-built-linux, linux-mediatek, LKML, Linux ARM
On Thu, Mar 7, 2019 at 2:57 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> used uninitialized whenever '||' condition is true
> [-Werror,-Wsometimes-uninitialized]
>
> If pwrap_write returns non-zero, pwrap_read will not be called to
> initialize rdata, meaning that we will use some random uninitialized
> stack value in our print statement. Zero initialize rdata in case this
> happens.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/401
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> I don't know if this is better or to just restructure the if statement
> below (I'm not an expert in this code so I'll leave that up to the
> maintainers to decide).
No, I the way you have it here is most correct. That condition writes
a value somewhere, reads it back, then compares it. The write or read
could fail. Better to just initialize rdata in case the write fails.
Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8236a6c87e19..2f632e8790f7 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> - u32 rdata;
> + u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
> --
> 2.21.0
>
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
2019-03-07 22:56 ` Nathan Chancellor
@ 2019-03-20 19:11 ` Nathan Chancellor
-1 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2019-03-20 19:11 UTC (permalink / raw)
To: Matthias Brugger
Cc: linux-arm-kernel, linux-mediatek, linux-kernel,
clang-built-linux, Nick Desaulniers
On Thu, Mar 07, 2019 at 03:56:51PM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> used uninitialized whenever '||' condition is true
> [-Werror,-Wsometimes-uninitialized]
>
> If pwrap_write returns non-zero, pwrap_read will not be called to
> initialize rdata, meaning that we will use some random uninitialized
> stack value in our print statement. Zero initialize rdata in case this
> happens.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/401
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> I don't know if this is better or to just restructure the if statement
> below (I'm not an expert in this code so I'll leave that up to the
> maintainers to decide).
>
> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8236a6c87e19..2f632e8790f7 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> - u32 rdata;
> + u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
> --
> 2.21.0
>
Gentle ping (if there was a response to this, I didn't receive it). I
know I sent it in the middle of a merge window so I get if it slipped
through the cracks.
Thanks,
Nathan
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-20 19:11 ` Nathan Chancellor
0 siblings, 0 replies; 12+ messages in thread
From: Nathan Chancellor @ 2019-03-20 19:11 UTC (permalink / raw)
To: Matthias Brugger
Cc: clang-built-linux, Nick Desaulniers, linux-mediatek,
linux-kernel, linux-arm-kernel
On Thu, Mar 07, 2019 at 03:56:51PM -0700, Nathan Chancellor wrote:
> When building with -Wsometimes-uninitialized, Clang warns:
>
> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> used uninitialized whenever '||' condition is true
> [-Werror,-Wsometimes-uninitialized]
>
> If pwrap_write returns non-zero, pwrap_read will not be called to
> initialize rdata, meaning that we will use some random uninitialized
> stack value in our print statement. Zero initialize rdata in case this
> happens.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/401
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> I don't know if this is better or to just restructure the if statement
> below (I'm not an expert in this code so I'll leave that up to the
> maintainers to decide).
>
> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index 8236a6c87e19..2f632e8790f7 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
> {
> int ret;
> - u32 rdata;
> + u32 rdata = 0;
>
> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
> --
> 2.21.0
>
Gentle ping (if there was a response to this, I didn't receive it). I
know I sent it in the middle of a merge window so I get if it slipped
through the cracks.
Thanks,
Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
2019-03-08 0:18 ` Nick Desaulniers
(?)
@ 2019-03-22 14:22 ` Arnd Bergmann
-1 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:22 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Matthias Brugger, Linux ARM,
moderated list:ARM/Mediatek SoC...,
LKML, clang-built-linux
On Fri, Mar 8, 2019 at 1:18 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Mar 7, 2019 at 2:57 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> > used uninitialized whenever '||' condition is true
> > [-Werror,-Wsometimes-uninitialized]
> >
> > If pwrap_write returns non-zero, pwrap_read will not be called to
> > initialize rdata, meaning that we will use some random uninitialized
> > stack value in our print statement. Zero initialize rdata in case this
> > happens.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/401
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > I don't know if this is better or to just restructure the if statement
> > below (I'm not an expert in this code so I'll leave that up to the
> > maintainers to decide).
>
> No, I the way you have it here is most correct. That condition writes
> a value somewhere, reads it back, then compares it. The write or read
> could fail. Better to just initialize rdata in case the write fails.
> Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
I also came up with a similar patch here, but I move the initialization
to just before the first use of the variable:
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..eb5035fd8ecd 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1355,6 +1355,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
pwrap_writel(wrp, 1, PWRAP_CIPHER_MODE);
/* Write Test */
+ rdata = 0;
if (pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
PWRAP_DEW_WRITE_TEST_VAL) ||
pwrap_read(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
Very little difference, so let's go with your patch
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-22 14:22 ` Arnd Bergmann
0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:22 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Matthias Brugger, Linux ARM,
moderated list:ARM/Mediatek SoC...,
LKML, clang-built-linux
On Fri, Mar 8, 2019 at 1:18 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Mar 7, 2019 at 2:57 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> > used uninitialized whenever '||' condition is true
> > [-Werror,-Wsometimes-uninitialized]
> >
> > If pwrap_write returns non-zero, pwrap_read will not be called to
> > initialize rdata, meaning that we will use some random uninitialized
> > stack value in our print statement. Zero initialize rdata in case this
> > happens.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/401
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > I don't know if this is better or to just restructure the if statement
> > below (I'm not an expert in this code so I'll leave that up to the
> > maintainers to decide).
>
> No, I the way you have it here is most correct. That condition writes
> a value somewhere, reads it back, then compares it. The write or read
> could fail. Better to just initialize rdata in case the write fails.
> Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
I also came up with a similar patch here, but I move the initialization
to just before the first use of the variable:
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..eb5035fd8ecd 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1355,6 +1355,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
pwrap_writel(wrp, 1, PWRAP_CIPHER_MODE);
/* Write Test */
+ rdata = 0;
if (pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
PWRAP_DEW_WRITE_TEST_VAL) ||
pwrap_read(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
Very little difference, so let's go with your patch
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-03-22 14:22 ` Arnd Bergmann
0 siblings, 0 replies; 12+ messages in thread
From: Arnd Bergmann @ 2019-03-22 14:22 UTC (permalink / raw)
To: Nick Desaulniers
Cc: LKML, clang-built-linux, moderated list:ARM/Mediatek SoC...,
Matthias Brugger, Nathan Chancellor, Linux ARM
On Fri, Mar 8, 2019 at 1:18 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Mar 7, 2019 at 2:57 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
> > used uninitialized whenever '||' condition is true
> > [-Werror,-Wsometimes-uninitialized]
> >
> > If pwrap_write returns non-zero, pwrap_read will not be called to
> > initialize rdata, meaning that we will use some random uninitialized
> > stack value in our print statement. Zero initialize rdata in case this
> > happens.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/401
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > I don't know if this is better or to just restructure the if statement
> > below (I'm not an expert in this code so I'll leave that up to the
> > maintainers to decide).
>
> No, I the way you have it here is most correct. That condition writes
> a value somewhere, reads it back, then compares it. The write or read
> could fail. Better to just initialize rdata in case the write fails.
> Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
I also came up with a similar patch here, but I move the initialization
to just before the first use of the variable:
diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c
b/drivers/soc/mediatek/mtk-pmic-wrap.c
index 8236a6c87e19..eb5035fd8ecd 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -1355,6 +1355,7 @@ static int pwrap_init_cipher(struct pmic_wrapper *wrp)
pwrap_writel(wrp, 1, PWRAP_CIPHER_MODE);
/* Write Test */
+ rdata = 0;
if (pwrap_write(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
PWRAP_DEW_WRITE_TEST_VAL) ||
pwrap_read(wrp, wrp->slave->dew_regs[PWRAP_DEW_WRITE_TEST],
Very little difference, so let's go with your patch
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
2019-03-20 19:11 ` Nathan Chancellor
@ 2019-04-12 19:58 ` Matthias Brugger
-1 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2019-04-12 19:58 UTC (permalink / raw)
To: Nathan Chancellor
Cc: linux-arm-kernel, linux-mediatek, linux-kernel,
clang-built-linux, Nick Desaulniers
On 20/03/2019 20:11, Nathan Chancellor wrote:
> On Thu, Mar 07, 2019 at 03:56:51PM -0700, Nathan Chancellor wrote:
>> When building with -Wsometimes-uninitialized, Clang warns:
>>
>> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
>> used uninitialized whenever '||' condition is true
>> [-Werror,-Wsometimes-uninitialized]
>>
>> If pwrap_write returns non-zero, pwrap_read will not be called to
>> initialize rdata, meaning that we will use some random uninitialized
>> stack value in our print statement. Zero initialize rdata in case this
>> happens.
>>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/401
>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> ---
>>
>> I don't know if this is better or to just restructure the if statement
>> below (I'm not an expert in this code so I'll leave that up to the
>> maintainers to decide).
>>
>> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> index 8236a6c87e19..2f632e8790f7 100644
>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
>> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>> {
>> int ret;
>> - u32 rdata;
>> + u32 rdata = 0;
>>
>> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
>> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
>> --
>> 2.21.0
>>
>
> Gentle ping (if there was a response to this, I didn't receive it). I
> know I sent it in the middle of a merge window so I get if it slipped
> through the cracks.
>
applied now to v5.1-next/soc
thanks
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher
@ 2019-04-12 19:58 ` Matthias Brugger
0 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2019-04-12 19:58 UTC (permalink / raw)
To: Nathan Chancellor
Cc: clang-built-linux, Nick Desaulniers, linux-mediatek,
linux-kernel, linux-arm-kernel
On 20/03/2019 20:11, Nathan Chancellor wrote:
> On Thu, Mar 07, 2019 at 03:56:51PM -0700, Nathan Chancellor wrote:
>> When building with -Wsometimes-uninitialized, Clang warns:
>>
>> drivers/soc/mediatek/mtk-pmic-wrap.c:1358:6: error: variable 'rdata' is
>> used uninitialized whenever '||' condition is true
>> [-Werror,-Wsometimes-uninitialized]
>>
>> If pwrap_write returns non-zero, pwrap_read will not be called to
>> initialize rdata, meaning that we will use some random uninitialized
>> stack value in our print statement. Zero initialize rdata in case this
>> happens.
>>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/401
>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> ---
>>
>> I don't know if this is better or to just restructure the if statement
>> below (I'm not an expert in this code so I'll leave that up to the
>> maintainers to decide).
>>
>> drivers/soc/mediatek/mtk-pmic-wrap.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> index 8236a6c87e19..2f632e8790f7 100644
>> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
>> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
>> @@ -1281,7 +1281,7 @@ static bool pwrap_is_pmic_cipher_ready(struct pmic_wrapper *wrp)
>> static int pwrap_init_cipher(struct pmic_wrapper *wrp)
>> {
>> int ret;
>> - u32 rdata;
>> + u32 rdata = 0;
>>
>> pwrap_writel(wrp, 0x1, PWRAP_CIPHER_SWRST);
>> pwrap_writel(wrp, 0x0, PWRAP_CIPHER_SWRST);
>> --
>> 2.21.0
>>
>
> Gentle ping (if there was a response to this, I didn't receive it). I
> know I sent it in the middle of a merge window so I get if it slipped
> through the cracks.
>
applied now to v5.1-next/soc
thanks
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-12 19:58 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-07 22:56 [PATCH] soc: mediatek: pwrap: Zero initialize rdata in pwrap_init_cipher Nathan Chancellor
2019-03-07 22:56 ` Nathan Chancellor
2019-03-07 22:56 ` Nathan Chancellor
2019-03-08 0:18 ` Nick Desaulniers
2019-03-08 0:18 ` Nick Desaulniers
2019-03-22 14:22 ` Arnd Bergmann
2019-03-22 14:22 ` Arnd Bergmann
2019-03-22 14:22 ` Arnd Bergmann
2019-03-20 19:11 ` Nathan Chancellor
2019-03-20 19:11 ` Nathan Chancellor
2019-04-12 19:58 ` Matthias Brugger
2019-04-12 19:58 ` Matthias Brugger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.