[PATCH] platform/x86/intel/tpmi/plr: Uninitialized variable in plr_print_bits()

[PATCH] platform/x86/intel/tpmi/plr: Uninitialized variable in plr_print_bits()

[Dan Carpenter]

Initialize the "str" pointer to NULL.  There is a test later for if "str"
is NULL but in the original code it was either valid or uninitialized.

Fixes: 9e9397a41b7b ("platform/x86/intel/tpmi/plr: Add support for the plr mailbox")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
Almost everyone automatically initializes stack variables to zero these days so
bugs like this don't show up in testing and we disabled GCC's uninitialized
variable warning so it's easy to miss.

 drivers/platform/x86/intel/intel_plr_tpmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/intel_plr_tpmi.c b/drivers/platform/x86/intel/intel_plr_tpmi.c
index c1aa52c23d25..2725a1ddba92 100644
--- a/drivers/platform/x86/intel/intel_plr_tpmi.c
+++ b/drivers/platform/x86/intel/intel_plr_tpmi.c
@@ -162,7 +162,7 @@ static int plr_clear_cpu_status(struct tpmi_plr_die *plr_die, int cpu)
 static void plr_print_bits(struct seq_file *s, u64 val, int bits)
 {
 	const unsigned long mask[] = { BITMAP_FROM_U64(val) };
-	const char *str;
+	const char *str = NULL;
 	int bit, index;
 
 	for_each_set_bit(bit, mask, bits) {
-- 
2.43.0

Re: [PATCH] platform/x86/intel/tpmi/plr: Uninitialized variable in plr_print_bits()

[Ilpo Järvinen]

On Fri, 12 Jul 2024, Dan Carpenter wrote:

> Initialize the "str" pointer to NULL.  There is a test later for if "str"
> is NULL but in the original code it was either valid or uninitialized.
> 
> Fixes: 9e9397a41b7b ("platform/x86/intel/tpmi/plr: Add support for the plr mailbox")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> Almost everyone automatically initializes stack variables to zero these days so
> bugs like this don't show up in testing and we disabled GCC's uninitialized
> variable warning so it's easy to miss.
> 
>  drivers/platform/x86/intel/intel_plr_tpmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/intel_plr_tpmi.c b/drivers/platform/x86/intel/intel_plr_tpmi.c
> index c1aa52c23d25..2725a1ddba92 100644
> --- a/drivers/platform/x86/intel/intel_plr_tpmi.c
> +++ b/drivers/platform/x86/intel/intel_plr_tpmi.c
> @@ -162,7 +162,7 @@ static int plr_clear_cpu_status(struct tpmi_plr_die *plr_die, int cpu)
>  static void plr_print_bits(struct seq_file *s, u64 val, int bits)
>  {
>  	const unsigned long mask[] = { BITMAP_FROM_U64(val) };
> -	const char *str;
> +	const char *str = NULL;
>  	int bit, index;
>  
>  	for_each_set_bit(bit, mask, bits) {

This fix looks slightly incorrect. It silences warning but for logic 
correctness, the NULL assignment seems to belong inside the for loop so 
it's done for each bit.

-- 
 i.

Re: [PATCH] platform/x86/intel/tpmi/plr: Uninitialized variable in plr_print_bits()

[Dan Carpenter]

On Mon, Jul 15, 2024 at 12:25:21PM +0300, Ilpo Järvinen wrote:
> On Fri, 12 Jul 2024, Dan Carpenter wrote:
> 
> > Initialize the "str" pointer to NULL.  There is a test later for if "str"
> > is NULL but in the original code it was either valid or uninitialized.
> > 
> > Fixes: 9e9397a41b7b ("platform/x86/intel/tpmi/plr: Add support for the plr mailbox")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> > Almost everyone automatically initializes stack variables to zero these days so
> > bugs like this don't show up in testing and we disabled GCC's uninitialized
> > variable warning so it's easy to miss.
> > 
> >  drivers/platform/x86/intel/intel_plr_tpmi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/platform/x86/intel/intel_plr_tpmi.c b/drivers/platform/x86/intel/intel_plr_tpmi.c
> > index c1aa52c23d25..2725a1ddba92 100644
> > --- a/drivers/platform/x86/intel/intel_plr_tpmi.c
> > +++ b/drivers/platform/x86/intel/intel_plr_tpmi.c
> > @@ -162,7 +162,7 @@ static int plr_clear_cpu_status(struct tpmi_plr_die *plr_die, int cpu)
> >  static void plr_print_bits(struct seq_file *s, u64 val, int bits)
> >  {
> >  	const unsigned long mask[] = { BITMAP_FROM_U64(val) };
> > -	const char *str;
> > +	const char *str = NULL;
> >  	int bit, index;
> >  
> >  	for_each_set_bit(bit, mask, bits) {
> 
> This fix looks slightly incorrect.

s/slightly/totally/.

I'll resend.

regards,
dan carpenter