| Message ID | 1538056115-6677-1-git-send-email-eranbe@mellanox.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | John Linville |
| Headers | show |
| Series | [ethtool] ethtool: Fix uninitialized variable use at qsfp dump | expand |
On Thu, Sep 27, 2018 at 04:48:35PM +0300, Eran Ben Elisha wrote: > Struct sff_diags can be used uninitialized at sff8636_show_dom, this > caused the tool to show unreported fields (supports_alarms) by the lower > level driver. > > Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support") > Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> > --- > qsfp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qsfp.c b/qsfp.c > index 32e195d12dc0..d196aa1753de 100644 > --- a/qsfp.c > +++ b/qsfp.c > @@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd) > > static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len) > { > - struct sff_diags sd; > + struct sff_diags sd = {0}; > char *rx_power_string = NULL; > char power_string[MAX_DESC_SIZE]; > int i; Hi Erin I wonder if similar bugs happen in sfpdiag.c? void sff8472_show_all(const __u8 *id) { struct sff_diags sd; char *rx_power_string = NULL; int i; sd is also on the stack and not initialised. Could you expand your patch to also clear this one as well, just to be safe. Thanks Andrew
On 9/27/2018 7:08 PM, Andrew Lunn wrote: > On Thu, Sep 27, 2018 at 04:48:35PM +0300, Eran Ben Elisha wrote: >> Struct sff_diags can be used uninitialized at sff8636_show_dom, this >> caused the tool to show unreported fields (supports_alarms) by the lower >> level driver. >> >> Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support") >> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> >> --- >> qsfp.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/qsfp.c b/qsfp.c >> index 32e195d12dc0..d196aa1753de 100644 >> --- a/qsfp.c >> +++ b/qsfp.c >> @@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd) >> >> static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len) >> { >> - struct sff_diags sd; >> + struct sff_diags sd = {0}; >> char *rx_power_string = NULL; >> char power_string[MAX_DESC_SIZE]; >> int i; > > Hi Erin > > I wonder if similar bugs happen in sfpdiag.c? Bug is not present here, all sd needed fields are being filled in sff8472_parse_eeprom right at the beginning. > > void sff8472_show_all(const __u8 *id) > { > struct sff_diags sd; > char *rx_power_string = NULL; > int i; > > sd is also on the stack and not initialised. Could you expand your > patch to also clear this one as well, just to be safe. Although bug is not present, I agree that better initialize it to be on the safe side. will post soon. Eran > > Thanks > Andrew >
diff --git a/qsfp.c b/qsfp.c index 32e195d12dc0..d196aa1753de 100644 --- a/qsfp.c +++ b/qsfp.c @@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd) static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len) { - struct sff_diags sd; + struct sff_diags sd = {0}; char *rx_power_string = NULL; char power_string[MAX_DESC_SIZE]; int i;
Struct sff_diags can be used uninitialized at sff8636_show_dom, this caused the tool to show unreported fields (supports_alarms) by the lower level driver. Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support") Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com> --- qsfp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)