[v1,04/12] net/ice: avoid reading past end of PFA

Message ID 20240822095612.216214-5-soumyadeep.hore@intel.com (mailing list archive)
State Superseded
Delegated to: Bruce Richardson
Headers
Series Align ICE shared code with Base driver |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Hore, Soumyadeep Aug. 22, 2024, 9:56 a.m. UTC
The ice_get_pfa_module_tlv() function iterates over the Preserved Fields
Area to read data from the Shadow RAM, including the Part Board Assembly
data, among others.

If the specific TLV being requested is not found in the current NVM, the
code will read past the end of the PFA, misinterpreting the last word of
the PFA and the word just after the PFA as another TLV. This typically
results in one extra iteration before the length check of the while loop is
triggered.

Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
---
 drivers/net/ice/base/ice_nvm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
  

Comments

Bruce Richardson Aug. 22, 2024, 2:41 p.m. UTC | #1
On Thu, Aug 22, 2024 at 09:56:04AM +0000, Soumyadeep Hore wrote:
> The ice_get_pfa_module_tlv() function iterates over the Preserved Fields
> Area to read data from the Shadow RAM, including the Part Board Assembly
> data, among others.
> 
> If the specific TLV being requested is not found in the current NVM, the
> code will read past the end of the PFA, misinterpreting the last word of
> the PFA and the word just after the PFA as another TLV. This typically
> results in one extra iteration before the length check of the while loop is
> triggered.
> 
> Signed-off-by: Soumyadeep Hore <soumyadeep.hore@intel.com>
> ---

This looks like a bug fix, so needs a fixes tag in V2.

/Bruce
>  drivers/net/ice/base/ice_nvm.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ice/base/ice_nvm.c b/drivers/net/ice/base/ice_nvm.c
> index 5e982de4b5..0124cef04c 100644
> --- a/drivers/net/ice/base/ice_nvm.c
> +++ b/drivers/net/ice/base/ice_nvm.c
> @@ -498,11 +498,16 @@ 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;
>  	}
> -	/* Starting with first TLV after PFA length, iterate through the list
> +	/* The Preserved Fields Area contains a sequence of TLVs which define
> +	 * its contents. The PFA length includes all of the TLVs, plus its
> +	 * initial length word itself, *and* one final word at the end of all
> +	 * of the TLVs.
> +	 *
> +	 * 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 < ((u32)pfa_ptr + pfa_len)) {
> +	while (next_tlv < ((u32)pfa_ptr + pfa_len - 1)) {
>  		u16 tlv_sub_module_type;
>  		u16 tlv_len;
>  
> -- 
> 2.43.0
>
  

Patch

diff --git a/drivers/net/ice/base/ice_nvm.c b/drivers/net/ice/base/ice_nvm.c
index 5e982de4b5..0124cef04c 100644
--- a/drivers/net/ice/base/ice_nvm.c
+++ b/drivers/net/ice/base/ice_nvm.c
@@ -498,11 +498,16 @@  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;
 	}
-	/* Starting with first TLV after PFA length, iterate through the list
+	/* The Preserved Fields Area contains a sequence of TLVs which define
+	 * its contents. The PFA length includes all of the TLVs, plus its
+	 * initial length word itself, *and* one final word at the end of all
+	 * of the TLVs.
+	 *
+	 * 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 < ((u32)pfa_ptr + pfa_len)) {
+	while (next_tlv < ((u32)pfa_ptr + pfa_len - 1)) {
 		u16 tlv_sub_module_type;
 		u16 tlv_len;