* [bug report] be2net: Fix buffer overflow in be_get_module_eeprom
@ 2022-07-21 12:36 Dan Carpenter
2022-07-22 15:20 ` [PATCH] be2net: Fix Smatch error Hristo Venev
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2022-07-21 12:36 UTC (permalink / raw)
To: hristo; +Cc: kernel-janitors
Hello Hristo Venev,
The patch d7241f679a59: "be2net: Fix buffer overflow in
be_get_module_eeprom" from Jul 16, 2022, leads to the following
Smatch static checker warning:
drivers/net/ethernet/emulex/benet/be_ethtool.c:1392 be_get_module_eeprom()
error: uninitialized symbol 'status'.
drivers/net/ethernet/emulex/benet/be_ethtool.c
1360 static int be_get_module_eeprom(struct net_device *netdev,
1361 struct ethtool_eeprom *eeprom, u8 *data)
1362 {
1363 struct be_adapter *adapter = netdev_priv(netdev);
1364 int status;
1365 u32 begin, end;
1366
1367 if (!check_privilege(adapter, MAX_PRIVILEGES))
1368 return -EOPNOTSUPP;
1369
1370 begin = eeprom->offset;
1371 end = eeprom->offset + eeprom->len;
1372
1373 if (begin < PAGE_DATA_LEN) {
1374 status = be_cmd_read_port_transceiver_data(adapter, TR_PAGE_A0, begin,
1375 min_t(u32, end, PAGE_DATA_LEN) - begin,
1376 data);
1377 if (status)
1378 goto err;
1379
1380 data += PAGE_DATA_LEN - begin;
1381 begin = PAGE_DATA_LEN;
1382 }
1383
1384 if (end > PAGE_DATA_LEN) {
1385 status = be_cmd_read_port_transceiver_data(adapter, TR_PAGE_A2,
1386 begin - PAGE_DATA_LEN,
1387 end - begin, data);
1388 if (status)
1389 goto err;
1390 }
status is not initalized if both conditions are false.
1391 err:
--> 1392 return be_cmd_status(status);
1393 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] be2net: Fix Smatch error
2022-07-21 12:36 [bug report] be2net: Fix buffer overflow in be_get_module_eeprom Dan Carpenter
@ 2022-07-22 15:20 ` Hristo Venev
2022-07-23 4:42 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Hristo Venev @ 2022-07-22 15:20 UTC (permalink / raw)
To: Dan Carpenter, Paolo Abeni; +Cc: netdev, kernel-janitors, Hristo Venev
drivers/net/ethernet/emulex/benet/be_ethtool.c:1392 be_get_module_eeprom()
error: uninitialized symbol 'status'.
When `eeprom->len == 0` and `eeprom->offset == PAGE_DATA_LEN`, we end
up with neither of the pages being read, so `status` is left
uninitialized.
While it appears that no caller will actually give `get_module_eeprom`
a zero length, fixing this issue is trivial.
Fixes: d7241f679a59 ("be2net: Fix buffer overflow in be_get_module_eeprom")
Signed-off-by: Hristo Venev <hristo@venev.name>
---
drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index bd0df189d871..2145882d00cc 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -1361,7 +1361,7 @@ static int be_get_module_eeprom(struct net_device *netdev,
struct ethtool_eeprom *eeprom, u8 *data)
{
struct be_adapter *adapter = netdev_priv(netdev);
- int status;
+ int status = 0;
u32 begin, end;
if (!check_privilege(adapter, MAX_PRIVILEGES))
--
2.37.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] be2net: Fix Smatch error
2022-07-22 15:20 ` [PATCH] be2net: Fix Smatch error Hristo Venev
@ 2022-07-23 4:42 ` Jakub Kicinski
2022-07-26 16:54 ` [PATCH net v2] be2net: Fix uninitialized variable Hristo Venev
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-07-23 4:42 UTC (permalink / raw)
To: Hristo Venev; +Cc: Dan Carpenter, Paolo Abeni, netdev, kernel-janitors
On Fri, 22 Jul 2022 18:20:52 +0300 Hristo Venev wrote:
> Subject: [PATCH] be2net: Fix Smatch error
Please describe the problem not the tool that found it, and name the
target tree in the tag ([PATCH net] in this case).
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net v2] be2net: Fix uninitialized variable
2022-07-23 4:42 ` Jakub Kicinski
@ 2022-07-26 16:54 ` Hristo Venev
2022-07-27 17:27 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Hristo Venev @ 2022-07-26 16:54 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Dan Carpenter, Paolo Abeni, netdev, kernel-janitors, Hristo Venev
The following error is reported by Smatch:
drivers/net/ethernet/emulex/benet/be_ethtool.c:1392 be_get_module_eeprom()
error: uninitialized symbol 'status'.
When `eeprom->len == 0` and `eeprom->begin == PAGE_DATA_LEN`, we end
up with neither of the pages being read, so `status` is left
uninitialized.
While it appears that no caller will actually give `get_module_eeprom`
a zero length, fixing this issue is trivial.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: d7241f679a59 ("be2net: Fix buffer overflow in be_get_module_eeprom")
Signed-off-by: Hristo Venev <hristo@venev.name>
---
drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index bd0df189d871..2145882d00cc 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -1361,7 +1361,7 @@ static int be_get_module_eeprom(struct net_device *netdev,
struct ethtool_eeprom *eeprom, u8 *data)
{
struct be_adapter *adapter = netdev_priv(netdev);
- int status;
+ int status = 0;
u32 begin, end;
if (!check_privilege(adapter, MAX_PRIVILEGES))
--
2.37.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] be2net: Fix uninitialized variable
2022-07-26 16:54 ` [PATCH net v2] be2net: Fix uninitialized variable Hristo Venev
@ 2022-07-27 17:27 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-07-27 17:27 UTC (permalink / raw)
To: Hristo Venev; +Cc: Dan Carpenter, Paolo Abeni, netdev, kernel-janitors
On Tue, 26 Jul 2022 19:54:54 +0300 Hristo Venev wrote:
> The following error is reported by Smatch:
>
> drivers/net/ethernet/emulex/benet/be_ethtool.c:1392 be_get_module_eeprom()
> error: uninitialized symbol 'status'.
>
> When `eeprom->len == 0` and `eeprom->begin == PAGE_DATA_LEN`, we end
> up with neither of the pages being read, so `status` is left
> uninitialized.
>
> While it appears that no caller will actually give `get_module_eeprom`
> a zero length, fixing this issue is trivial.
If there is no caller that can trigger this - it's not a fix. Fixes are
for bugs which can be triggered. Please repost against net-next without
the Fixes tag. Please don't post the v3 in reply to v2, just add a
changelog under the --- marker and make a fresh thread.
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Fixes: d7241f679a59 ("be2net: Fix buffer overflow in be_get_module_eeprom")
> Signed-off-by: Hristo Venev <hristo@venev.name>
> ---
> drivers/net/ethernet/emulex/benet/be_ethtool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
> index bd0df189d871..2145882d00cc 100644
> --- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
> +++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
> @@ -1361,7 +1361,7 @@ static int be_get_module_eeprom(struct net_device *netdev,
> struct ethtool_eeprom *eeprom, u8 *data)
> {
> struct be_adapter *adapter = netdev_priv(netdev);
> - int status;
> + int status = 0;
> u32 begin, end;
>
> if (!check_privilege(adapter, MAX_PRIVILEGES))
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-27 18:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-21 12:36 [bug report] be2net: Fix buffer overflow in be_get_module_eeprom Dan Carpenter
2022-07-22 15:20 ` [PATCH] be2net: Fix Smatch error Hristo Venev
2022-07-23 4:42 ` Jakub Kicinski
2022-07-26 16:54 ` [PATCH net v2] be2net: Fix uninitialized variable Hristo Venev
2022-07-27 17:27 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).