Skip to content

Commit 631ad94

Browse files
jacob-kellerSasha Levin
authored andcommitted
ice: fix iteration of TLVs in Preserved Fields Area
[ Upstream commit 03e4a09 ] The ice_get_pfa_module_tlv() function iterates over the Type-Length-Value structures in the Preserved Fields Area (PFA) of the NVM. This is used by the driver to access data such as the Part Board Assembly identifier. The function uses simple logic to iterate over the PFA. First, the pointer to the PFA in the NVM is read. Then the total length of the PFA is read from the first word. A pointer to the first TLV is initialized, and a simple loop iterates over each TLV. The pointer is moved forward through the NVM until it exceeds the PFA area. The logic seems sound, but it is missing a key detail. The Preserved Fields Area length includes one additional final word. This is documented in the device data sheet as a dummy word which contains 0xFFFF. All NVMs have this extra word. If the driver tries to scan for a TLV that is not in the PFA, it will read past the size of the PFA. It reads and interprets the last dummy word of the PFA as a TLV with type 0xFFFF. It then reads the word following the PFA as a length. The PFA resides within the Shadow RAM portion of the NVM, which is relatively small. All of its offsets are within a 16-bit size. The PFA pointer and TLV pointer are stored by the driver as 16-bit values. In almost all cases, the word following the PFA will be such that interpreting it as a length will result in 16-bit arithmetic overflow. Once overflowed, the new next_tlv value is now below the maximum offset of the PFA. Thus, the driver will continue to iterate the data as TLVs. In the worst case, the driver hits on a sequence of reads which loop back to reading the same offsets in an endless loop. To fix this, we need to correct the loop iteration check to account for this extra word at the end of the PFA. This alone is sufficient to resolve the known cases of this issue in the field. However, it is plausible that an NVM could be misconfigured or have corrupt data which results in the same kind of overflow. Protect against this by using check_add_overflow when calculating both the maximum offset of the TLVs, and when calculating the next_tlv offset at the end of each loop iteration. This ensures that the driver will not get stuck in an infinite loop when scanning the PFA. Fixes: e961b67 ("ice: add board identifier info to devlink .info_get") Co-developed-by: Paul Greenwalt <paul.greenwalt@intel.com> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://lore.kernel.org/r/20240603-net-2024-05-30-intel-net-fixes-v2-1-e3563aa89b0c@intel.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 8b114dc commit 631ad94

File tree

1 file changed

+21
-7
lines changed

1 file changed

+21
-7
lines changed

drivers/net/ethernet/intel/ice/ice_nvm.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,7 @@ int
441441
ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
442442
u16 module_type)
443443
{
444-
u16 pfa_len, pfa_ptr;
445-
u16 next_tlv;
444+
u16 pfa_len, pfa_ptr, next_tlv, max_tlv;
446445
int status;
447446

448447
status = ice_read_sr_word(hw, ICE_SR_PFA_PTR, &pfa_ptr);
@@ -455,11 +454,23 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
455454
ice_debug(hw, ICE_DBG_INIT, "Failed to read PFA length.\n");
456455
return status;
457456
}
457+
458+
/* The Preserved Fields Area contains a sequence of Type-Length-Value
459+
* structures which define its contents. The PFA length includes all
460+
* of the TLVs, plus the initial length word itself, *and* one final
461+
* word at the end after all of the TLVs.
462+
*/
463+
if (check_add_overflow(pfa_ptr, pfa_len - 1, &max_tlv)) {
464+
dev_warn(ice_hw_to_dev(hw), "PFA starts at offset %u. PFA length of %u caused 16-bit arithmetic overflow.\n",
465+
pfa_ptr, pfa_len);
466+
return -EINVAL;
467+
}
468+
458469
/* Starting with first TLV after PFA length, iterate through the list
459470
* of TLVs to find the requested one.
460471
*/
461472
next_tlv = pfa_ptr + 1;
462-
while (next_tlv < pfa_ptr + pfa_len) {
473+
while (next_tlv < max_tlv) {
463474
u16 tlv_sub_module_type;
464475
u16 tlv_len;
465476

@@ -483,10 +494,13 @@ ice_get_pfa_module_tlv(struct ice_hw *hw, u16 *module_tlv, u16 *module_tlv_len,
483494
}
484495
return -EINVAL;
485496
}
486-
/* Check next TLV, i.e. current TLV pointer + length + 2 words
487-
* (for current TLV's type and length)
488-
*/
489-
next_tlv = next_tlv + tlv_len + 2;
497+
498+
if (check_add_overflow(next_tlv, 2, &next_tlv) ||
499+
check_add_overflow(next_tlv, tlv_len, &next_tlv)) {
500+
dev_warn(ice_hw_to_dev(hw), "TLV of type %u and length 0x%04x caused 16-bit arithmetic overflow. The PFA starts at 0x%04x and has length of 0x%04x\n",
501+
tlv_sub_module_type, tlv_len, pfa_ptr, pfa_len);
502+
return -EINVAL;
503+
}
490504
}
491505
/* Module does not exist */
492506
return -ENOENT;

0 commit comments

Comments
 (0)