[Intel-wired-lan] [PATCH iwl-net v3] ice: avoid infinite loop if NVM has invalid TLV length

[Jacob Keller]

The ice_get_pfa_module_tlv() function iterates over the TLVs in the
Preserved Fields Area (PFA) of the NVM. This is used to access data such as
the Part Board Assembly identifier.

Some NVMs in the wild have been found with incorrect TLV lengths including
at least one which reports a TLV length of 0xFFFF. When trying to read the
PBA from such an NVM, the driver will compute a new offset for the next_tlv
which is lower, due to overflowing the 16-bit next_tlv variable.

In the best case, the driver will incorrectly interpret values until it
finds one which has an offset greater than the PFA area without
overflowing. In the worst case, the values in the NVM result in an infinite
loop as the misinterpreted lengths result in checking offsets which are
valid within the PFA, and which ultimately point in an infinite loop.

Fix this by using check_add_overflow when calculating the NVM offsets, and
bailing if we ever overflow. Additionally, use check_add_overflow when
calculating the initial maximum PFA size.

This ensures that we bail immediately on encountering any TLV who's length
would have caused the naive addition to overflow, rather than entering an
infinite loop or otherwise misinterpreting NVM values.

Fixes: e961b679fb0b ("ice: add board identifier info to devlink .info_get")
Co-developed-by: Paul Greenwalt <paul.greenwalt at intel.com>
Signed-off-by: Paul Greenwalt <paul.greenwalt at intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
---
Changes in v3:
- Fix missing {
- Fix missing pfa_ptr variable to dev_warn()
- Add Fixes tag
- Link to v2: https://lore.kernel.org/r/20240517-iwl-net-2024-05-16-fix-nvm-tlv-issue-v2-1-fdee184ece86@intel.com

Changes in v2:
- Use check_add_overflow instead of increasing the variables to u32
- Upgrade log messages to dev_warn()
- Link to v1: https://lore.kernel.org/r/20240516-iwl-net-2024-05-16-fix-nvm-tlv-issue-v1-1-ecbb6a75961e@intel.com
---
 drivers/net/ethernet/intel/ice/ice_nvm.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_nvm.c b/drivers/net/ethernet/intel/ice/ice_nvm.c
index 84eab92dc03c..be731b83d667 100644
--- a/drivers/net/ethernet/intel/ice/ice_nvm.c
+++ b/drivers/net/ethernet/intel/ice/ice_nvm.c
@@ -440,8 +440,7 @@ int
 ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
 		       u16 module_type)
 {
-	u16 pfa_len, pfa_ptr;
-	u16 next_tlv;
+	u16 pfa_len, pfa_ptr, next_tlv, max_pfa;
 	int status;
 
 	status = ice_read_sr_word(hw, ICE_SR_PFA_PTR, &pfa_ptr);
@@ -454,11 +453,18 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
 		ice_debug(hw, ICE_DBG_INIT, "Failed to read PFA length.\n");
 		return status;
 	}
+
+	if (check_add_overflow(pfa_ptr, pfa_len, &max_pfa)) {
+		dev_warn(ice_hw_to_dev(hw), "PFA starts at offset %u. PFA length of %u causes 16-bit arithmetic overflow.\n",
+			 pfa_ptr, pfa_len);
+		return -EINVAL;
+	}
+
 	/* Starting with first TLV after PFA length, iterate through the list
 	 * of TLVs to find the requested one.
 	 */
 	next_tlv = pfa_ptr + 1;
-	while (next_tlv < pfa_ptr + pfa_len) {
+	while (next_tlv < max_pfa) {
 		u16 tlv_sub_module_type;
 		u16 tlv_len;
 
@@ -485,7 +491,12 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
 		/* Check next TLV, i.e. current TLV pointer + length + 2 words
 		 * (for current TLV's type and length)
 		 */
-		next_tlv = next_tlv + tlv_len + 2;
+		if (check_add_overflow(next_tlv, 2, &next_tlv) ||
+		    check_add_overflow(next_tlv, tlv_len, &next_tlv)) {
+			dev_warn(ice_hw_to_dev(hw), "Failed to locate PFA TLV module of type %u due to arithmetic overflow. The PFA length is %u. The last TLV has length of %u\n",
+				 module_type, pfa_len, tlv_len);
+			return -EINVAL;
+		}
 	}
 	/* Module does not exist */
 	return -ENOENT;

---
base-commit: 83e93942796db58652288f0391ac00072401816f
change-id: 20240516-iwl-net-2024-05-16-fix-nvm-tlv-issue-99ebb2c55c52

Best regards,
-- 
Jacob Keller <jacob.e.keller at intel.com>