Skip to contentSkip to conversation

net/ice: fix trailing bytes drop in get_module_eeprom()

PATCH Superseded Marcel Telka 2 participants 4 messages Nov 12 – Nov 13 Network drivers

lore
Share & tools
lore

Marcel Telka

author
#0 lore raw

PATCH 1/1 net/ice: fix trailing bytes drop in get_module_eeprom()

When info->length is not an exact multiple of SFF_READ_BLOCK_SIZE (8), the final loop iteration reads the correct 8 bytes from the module into value[], but the guard evaluates false, so the memcpy is skipped entirely. Since the destination buffer is memset to 0 at function entry, the trailing length % 8 bytes are silently returned as zero instead of the real EEPROM content, with no error indication to the caller.

Bugzilla ID: 2091

Signed-off-by Marcel Telka <[email protected]>
---
 drivers/net/intel/ice/ice_ethdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/net/intel/ice/ice_ethdev.c +8 -3
index 457c955b2e..3de9840e7c 100644
@@ -6750,6 +6750,8 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 		 * SFP modules only ever use page 0.
 		 */
 		if (page == 0 || !(data[0x2] & 0x4)) {
+			uint32_t 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.

@@ -6777,9 +6779,12 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 				break;
 			}
 
-			/* Make sure we have enough room for the new block */
-			if ((i + SFF_READ_BLOCK_SIZE) <= info->length)
-				memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
+			/* Clamp the copy to the bytes actually requested: the final
+			 * block is shorter than SFF_READ_BLOCK_SIZE whenever
+			 * info->length is not a multiple of it.
+			 */
+			copy_len = RTE_MIN((uint32_t)SFF_READ_BLOCK_SIZE, info->length - i);
+			memcpy(data + i, value, copy_len);
 		}
 	}
author
#1 lore raw

PATCH v2 1/1 net/ice: fix trailing bytes drop in get_module_eeprom()

When info->length is not an exact multiple of SFF_READ_BLOCK_SIZE (8), the final loop iteration reads the correct 8 bytes from the module into value[], but the guard evaluates false, so the memcpy is skipped entirely. Since the destination buffer is memset to 0 at function entry, the trailing length % 8 bytes are silently returned as zero instead of the real EEPROM content, with no error indication to the caller.

Bugzilla ID: 2091

Fixes 5256925658 (net/ice: support module EEPROM)
Cc [email protected]
Signed-off-by Marcel Telka <[email protected]>
---
 drivers/net/intel/ice/ice_ethdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/net/intel/ice/ice_ethdev.c +8 -3
index 457c955b2e..3de9840e7c 100644
@@ -6750,6 +6750,8 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 		 * SFP modules only ever use page 0.
 		 */
 		if (page == 0 || !(data[0x2] & 0x4)) {
+			uint32_t 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.

@@ -6777,9 +6779,12 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 				break;
 			}
 
-			/* Make sure we have enough room for the new block */
-			if ((i + SFF_READ_BLOCK_SIZE) <= info->length)
-				memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
+			/* Clamp the copy to the bytes actually requested: the final
+			 * block is shorter than SFF_READ_BLOCK_SIZE whenever
+			 * info->length is not a multiple of it.
+			 */
+			copy_len = RTE_MIN((uint32_t)SFF_READ_BLOCK_SIZE, info->length - i);
+			memcpy(data + i, value, copy_len);
 		}
 	}
author
#2 lore raw

PATCH v3 1/1 net/ice: fix trailing bytes drop in get_module_eeprom()

When info->length is not an exact multiple of SFF_READ_BLOCK_SIZE (8), the final loop iteration reads the correct 8 bytes from the module into value[], but the guard evaluates false, so the memcpy is skipped entirely. Since the destination buffer is memset to 0 at function entry, the trailing length % 8 bytes are silently returned as zero instead of the real EEPROM content, with no error indication to the caller.

Bugzilla ID: 2091

Fixes 52569256583a ("net/ice: support module EEPROM")
Cc [email protected]
Signed-off-by Marcel Telka <[email protected]>
---
 drivers/net/intel/ice/ice_ethdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/net/intel/ice/ice_ethdev.c +8 -3
index 457c955b2e..3de9840e7c 100644
@@ -6750,6 +6750,8 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 		 * SFP modules only ever use page 0.
 		 */
 		if (page == 0 || !(data[0x2] & 0x4)) {
+			uint32_t 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.

@@ -6777,9 +6779,12 @@ ice_get_module_eeprom(struct rte_eth_dev *dev,
 				break;
 			}
 
-			/* Make sure we have enough room for the new block */
-			if ((i + SFF_READ_BLOCK_SIZE) <= info->length)
-				memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
+			/* Clamp the copy to the bytes actually requested: the final
+			 * block is shorter than SFF_READ_BLOCK_SIZE whenever
+			 * info->length is not a multiple of it.
+			 */
+			copy_len = RTE_MIN((uint32_t)SFF_READ_BLOCK_SIZE, info->length - i);
+			memcpy(data + i, value, copy_len);
 		}
 	}
Acked-by on v3 1/1 Applied
#3 lore raw
Quoted 7 lines · Marcel Telka

When info->length is not an exact multiple of SFF_READ_BLOCK_SIZE (8), the final loop iteration reads the correct 8 bytes from the module into value[], but the guard evaluates false, so the memcpy is skipped entirely. Since the destination buffer is memset to 0 at function entry, the trailing length % 8 bytes are silently returned as zero instead of the real EEPROM content, with no error indication to the caller.

Quoted 9 lines

Bugzilla ID: 2091

Fixes 52569256583a ("net/ice: support module EEPROM")
Cc [email protected]
Signed-off-by Marcel Telka <[email protected]>
---
 drivers/net/intel/ice/ice_ethdev.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Acked-by Bruce Richardson <[email protected]>

Applied to dpdk-next-net-intel.
Thanks,
/Bruce