| Message ID | 20230329162629.96590-1-den-plotnikov@yandex-team.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: netxen: report error on version offset reading | expand |
On Wed, Mar 29, 2023 at 07:26:29PM +0300, Denis Plotnikov wrote: > A static analyzer complains for non-checking the function returning value. > Although, the code looks like not expecting any problems with version > reading on netxen_p3_has_mn call, it seems the error still may happen. > So, at least, add error reporting to ease problems investigation. > > Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru> > --- > drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c > index 35ec9aab3dc7b..92962dbb73ad0 100644 > --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c > +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c > @@ -1192,8 +1192,13 @@ netxen_p3_has_mn(struct netxen_adapter *adapter) > if (NX_IS_REVISION_P2(adapter->ahw.revision_id)) > return 1; > > - netxen_rom_fast_read(adapter, > - NX_FW_VERSION_OFFSET, (int *)&flashed_ver); > + if (netxen_rom_fast_read(adapter, > + NX_FW_VERSION_OFFSET, (int *)&flashed_ver)) { 1. Mo callers of netxen_rom_fast_read() print debug messages, so this shouldn't too. 2. netxen_p3_has_mn() can't fail and by returning 0, you will cause to unpredictable behaviour in netxen_validate_firmware(). Thanks > + printk(KERN_ERR "%s: ERROR on flashed version reading", > + netxen_nic_driver_name); > + return 0; > + } > + > flashed_ver = NETXEN_DECODE_VERSION(flashed_ver); > > if (flashed_ver >= NETXEN_VERSION_CODE(4, 0, 220)) { > -- > 2.25.1 >
On 29.03.2023 21:59, Leon Romanovsky wrote: > On Wed, Mar 29, 2023 at 07:26:29PM +0300, Denis Plotnikov wrote: >> A static analyzer complains for non-checking the function returning value. >> Although, the code looks like not expecting any problems with version >> reading on netxen_p3_has_mn call, it seems the error still may happen. >> So, at least, add error reporting to ease problems investigation. >> >> Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru> >> --- >> drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c >> index 35ec9aab3dc7b..92962dbb73ad0 100644 >> --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c >> +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c >> @@ -1192,8 +1192,13 @@ netxen_p3_has_mn(struct netxen_adapter *adapter) >> if (NX_IS_REVISION_P2(adapter->ahw.revision_id)) >> return 1; >> >> - netxen_rom_fast_read(adapter, >> - NX_FW_VERSION_OFFSET, (int *)&flashed_ver); >> + if (netxen_rom_fast_read(adapter, >> + NX_FW_VERSION_OFFSET, (int *)&flashed_ver)) { > 1. Mo callers of netxen_rom_fast_read() print debug messages, so this > shouldn't too. > 2. netxen_p3_has_mn() can't fail and by returning 0, you will cause to > unpredictable behaviour in netxen_validate_firmware(). > > Thanks Well, ok. Then patch isn't needed. Thanks for reviewing! Denis > >> + printk(KERN_ERR "%s: ERROR on flashed version reading", >> + netxen_nic_driver_name); >> + return 0; >> + } >> + >> flashed_ver = NETXEN_DECODE_VERSION(flashed_ver); >> >> if (flashed_ver >= NETXEN_VERSION_CODE(4, 0, 220)) { >> -- >> 2.25.1 >>
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c index 35ec9aab3dc7b..92962dbb73ad0 100644 --- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c +++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c @@ -1192,8 +1192,13 @@ netxen_p3_has_mn(struct netxen_adapter *adapter) if (NX_IS_REVISION_P2(adapter->ahw.revision_id)) return 1; - netxen_rom_fast_read(adapter, - NX_FW_VERSION_OFFSET, (int *)&flashed_ver); + if (netxen_rom_fast_read(adapter, + NX_FW_VERSION_OFFSET, (int *)&flashed_ver)) { + printk(KERN_ERR "%s: ERROR on flashed version reading", + netxen_nic_driver_name); + return 0; + } + flashed_ver = NETXEN_DECODE_VERSION(flashed_ver); if (flashed_ver >= NETXEN_VERSION_CODE(4, 0, 220)) {
A static analyzer complains for non-checking the function returning value. Although, the code looks like not expecting any problems with version reading on netxen_p3_has_mn call, it seems the error still may happen. So, at least, add error reporting to ease problems investigation. Signed-off-by: Denis Plotnikov <den-plotnikov@yandex-team.ru> --- drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)