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

[Bruce Richardson <bruce.richardson@intel.com>]

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
>