| Message ID | 8ccfab0c-3c11-4168-a383-19895ae60022@stanley.mountain (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86/intel/tpmi/plr: Uninitialized variable in plr_print_bits() | expand |
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.
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
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) {
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(-)