ice_get_module_eeprom() is broken since commit e9c9692c8a81 ("ice:
Reimplement module reads used by ethtool") In this refactor,
ice_get_module_eeprom() reads the eeprom in blocks of size 8.
But the condition that should protect the buffer overflow
ignores the last block. The last block always contains zeros.
Bug uncovered by ethtool upstream commit 9538f384b535
("netlink: eeprom: Defer page requests to individual parsers")
After this commit, ethtool reads a block with length = 1;
to read the SFF-8024 identifier value.
unpatched driver:
$ ethtool -m enp65s0f0np0 offset 0x90 length 8
Offset Values
------ ------
0x0090: 00 00 00 00 00 00 00 00
$ ethtool -m enp65s0f0np0 offset 0x90 length 12
Offset Values
------ ------
0x0090: 00 00 01 a0 4d 65 6c 6c 00 00 00 00
$
$ ethtool -m enp65s0f0np0
Offset Values
------ ------
0x0000: 11 06 06 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 01 08 00
0x0070: 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00
patched driver:
$ ethtool -m enp65s0f0np0 offset 0x90 length 8
Offset Values
------ ------
0x0090: 00 00 01 a0 4d 65 6c 6c
$ ethtool -m enp65s0f0np0 offset 0x90 length 12
Offset Values
------ ------
0x0090: 00 00 01 a0 4d 65 6c 6c 61 6e 6f 78
$ ethtool -m enp65s0f0np0
Identifier : 0x11 (QSFP28)
Extended identifier : 0x00
Extended identifier description : 1.5W max. Power consumption
Extended identifier description : No CDR in TX, No CDR in RX
Extended identifier description : High Power Class (> 3.5 W) not enabled
Connector : 0x23 (No separable connector)
Transceiver codes : 0x88 0x00 0x00 0x00 0x00 0x00 0x00 0x00
Transceiver type : 40G Ethernet: 40G Base-CR4
Transceiver type : 25G Ethernet: 25G Base-CR CA-N
Encoding : 0x05 (64B/66B)
BR, Nominal : 25500Mbps
Rate identifier : 0x00
Length (SMF,km) : 0km
Length (OM3 50um) : 0m
Length (OM2 50um) : 0m
Length (OM1 62.5um) : 0m
Length (Copper or Active cable) : 1m
Transmitter technology : 0xa0 (Copper cable unequalized)
Attenuation at 2.5GHz : 4db
Attenuation at 5.0GHz : 5db
Attenuation at 7.0GHz : 7db
Attenuation at 12.9GHz : 10db
........
....
Fixes: e9c9692c8a81 ("ice: Reimplement module reads used by ethtool")
Signed-off-by: Petr Oros <[email protected]>
---
v2: memcpy unified calls
v3: copy_len is now declared in if scope
unwrapped line before memcpy
---
---
drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index b360bd8f15998b..f86e814354a311 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -4331,6 +4331,8 @@ ice_get_module_eeprom(struct net_device *netdev,
* SFP modules only ever use page 0.
*/
if (page == 0 || !(data[0x2] & 0x4)) {
+ u32 copy_len;
+
/* If i2c bus is busy due to slow page change or
* link management access, call can fail. This is normal.
* So we retry this a few times.
@@ -4354,8 +4356,8 @@ ice_get_module_eeprom(struct net_device *netdev,
}
/* Make sure we have enough room for the new block */
- if ((i + SFF_READ_BLOCK_SIZE) < ee->len)
- memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
+ copy_len = min_t(u32, SFF_READ_BLOCK_SIZE, ee->len - i);
+ memcpy(data + i, value, copy_len);
}
}
return 0;
--
2.39.2
On 3/1/2023 12:47 PM, Petr Oros wrote:
> ice_get_module_eeprom() is broken since commit e9c9692c8a81 ("ice:
> Reimplement module reads used by ethtool") In this refactor,
> ice_get_module_eeprom() reads the eeprom in blocks of size 8.
> But the condition that should protect the buffer overflow
> ignores the last block. The last block always contains zeros.
...
>
> Fixes: e9c9692c8a81 ("ice: Reimplement module reads used by ethtool")
> Signed-off-by: Petr Oros <[email protected]>
Reviewed-by: Jesse Brandeburg <[email protected]>
Tested-by: Jesse Brandeburg <[email protected]>
Thanks Petr!
From: Petr Oros <[email protected]>
Date: Wed, 1 Mar 2023 21:47:07 +0100
> ice_get_module_eeprom() is broken since commit e9c9692c8a81 ("ice:
> Reimplement module reads used by ethtool") In this refactor,
> ice_get_module_eeprom() reads the eeprom in blocks of size 8.
> But the condition that should protect the buffer overflow
> ignores the last block. The last block always contains zeros.
[...]
> Fixes: e9c9692c8a81 ("ice: Reimplement module reads used by ethtool")
> Signed-off-by: Petr Oros <[email protected]>
Reviewed-by: Alexander Lobakin <[email protected]>
> ---
> v2: memcpy unified calls
> v3: copy_len is now declared in if scope
> unwrapped line before memcpy
[...]
Thanks,
Olek