Message-ID: <d062a735-ed89-d437-16cc-ca7456c7de08@intel.com>
Date:   Wed, 1 Mar 2023 11:26:55 -0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH net v2] ice: copy last block omitted in
 ice_get_module_eeprom()
To:     Petr Oros <poros@redhat.com>, <netdev@vger.kernel.org>
CC:     <aleksander.lobakin@intel.com>, <anthony.l.nguyen@intel.com>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>, <scott.w.taylor@intel.com>,
        <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>
References: <20230228204139.2264495-1-poros@redhat.com>
 <20230301171452.2514734-1-poros@redhat.com>
Content-Language: en-US
From:   Jesse Brandeburg <jesse.brandeburg@intel.com>
In-Reply-To: <20230301171452.2514734-1-poros@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZGd0ZFJIcDErRHhubG5MWDlZdWRDYW85WUFXY3NIOGYvc05nTTBJWFg0S3JC?=
 =?utf-8?B?di9odFJmWVdqZTdhc2FWRGt0WE0yQ0hEZjdWeGthKzRBODI1UWNraHlzNHR0?=
 =?utf-8?B?Tlp5blhwWVFIYzhpZ2NJWWllY24wSWtyZUkySEwvazhzemZQWnhQYzF3QUlR?=
 =?utf-8?B?ME5oUlRaNkZraFZFL1NMR1BsRFlqQUpRN05wbXRuT2FocmtkNzUxenlyR3Bl?=
 =?utf-8?B?S1VIWVFGVTJ4SjFYWWtjc3lzWEw5TmRkbW1DU0ZaWlpVWlpTcVVRZEgyOFBa?=
 =?utf-8?B?K0lFUnFqbzZ3VW5HQjl4dGt3Sys1OTlOaFprYW9VWnorV2drMHhwQiszU0dh?=
 =?utf-8?B?S0c4ZzVWaTArYUROUkdTQnBtMTBoaUJ2VVRXSWQ5RXFQWW4rTDlxa1V5cDZN?=
 =?utf-8?B?aFdGMDE2UzNIUmhMempGRE9XeVRDMkI4YXZvSjZJTTJyejNyRnJmTzVLRTZP?=
 =?utf-8?B?VjF1dVJYa3ViUmN6M2dlSHVRUmxZaDMrdHEzeUtSazI3L2FGYjVKVXF0VFEx?=
 =?utf-8?B?c3VWam5idS9RbFNaOFo4akdZRWhmYnBqVURPdTJKMlIrZndhNkREdEkrWUk5?=
 =?utf-8?B?TTdvajFob216cExia1BTRGNSYTRFUW8vd29ydWMyRVRqZElNSUw4Tm1LMmNN?=
 =?utf-8?B?NVNKT09GaWZsMWRBdmpPTGlEeFJ5NlVoZ3dQb0hzMklxWWhYbGltQTlibHZD?=
 =?utf-8?B?MllQMDlUb0owQnVSOHRSdUMyMUFjZVg1RlVhT1krTGNzdVd6NGd1Ti9EYWY3?=
 =?utf-8?B?YmxyVTRNUDc3djh2NytaeUVTdk1oVlVsUGdzeDE5L2hxcW55RFJBM2Z3cXo0?=
 =?utf-8?B?NkNFUUE0Rm05T2hHaXY1eUxXMkhMRWkxUm1kVmpIYUtidmtzQWpEdGpwSGVC?=
 =?utf-8?B?S2JBZmdnd3lqajZaQ3o0ZisraUFTWWc5Qjc1ZHcxQjFLN0JPdm0rd3JlbEg2?=
 =?utf-8?B?RTdoVE94WXM4R2VzTmdSUjFJNk5pVlpXMHZaOHB6VzJ3NjZ0VHRqSjNYbjd6?=
 =?utf-8?B?aCs2cmJxM1h5RDNza3dQWFVyc0Y0bWpXRlQrRFU0bFlhSUs0c1FhQzM3OCtZ?=
 =?utf-8?B?MXBHbUdPR3ZNVndiL3JNUWFUZnVZSjdLUWIzNkltdWR5VGpUWE9nVnFPWWg3?=
 =?utf-8?B?ejVDWDhQU2tjNXdZSEJSQ3ltWFBUSmtHRHNFNWE0TEkyWGZRallXUTlObHox?=
 =?utf-8?B?SGlYa1FBb1VCTlBnV3JiZXRlOUJhdU0yZm5lT0NjNmFRQmxJVERMSEJSQmNj?=
 =?utf-8?B?SE1ycThMYm5ucVcxUlJ5YnVPV2Y5K2NZdzA2eU5WK1JONVpPaGVoMHY1ZVgz?=
 =?utf-8?B?eTZiUWVHRlhzVmZwNUw2d1B2VUgxRUJLczFJRUNVcUxvS29rSXFDc3VhMEdI?=
 =?utf-8?B?TTFGczBrTHZxOFlEY2xwYXpLM2xGNWsxRURKN29PdlVrSXVPeWdYbGNiQ2w0?=
 =?utf-8?B?UEVWQUtLMjdQVE1WR2tpdThieENGbVZEWm1pTUl5WHpvUmFuMjJzd2xtaUxq?=
 =?utf-8?B?UmJRUFVTRUpPUGdkeEsvZGx4OE4zQW1Pbm5zNWNIM2I3MEtLempiVnA0VXRK?=
 =?utf-8?B?Y1owQUdmNUNLQXFjczhkZ3V4cUpaNW10VVRzNlZzMU1NcC9uNTVSS1dXQXlU?=
 =?utf-8?B?dDRXUG9tSnYyTG5JNUQwL2ZBMDFYUElPdFdVUUEwalBBbFM1aGhJUG1abG96?=
 =?utf-8?B?eWxIeE1PSFdOQm45UTFYeGwrV1ZrcmlKMzJrTHpIMEY0NVhIVXZ0c051MFBN?=
 =?utf-8?B?NExvbmJZYzVDSzI0NjN2OTlkODExa2hZeUg0aWFScDRGT0dFbGszUmdpL2Q1?=
 =?utf-8?B?WVdBazl4NXlSRnNGZXNRU05NT2xoNjNXUUt3c0I2M0tmUit1R3l6cGVoaExZ?=
 =?utf-8?B?SVNCNXN0ZjB3U05PMUhzL2lldjMyL2hGTDdVU01kdWR6eFFNZjcvZkRDQ0Yv?=
 =?utf-8?B?SjVVbVFyMjZFZGx0SGVHSDhYUzVWVUtrNCtmdGJCclYvMU9XT21sblgrWUJw?=
 =?utf-8?B?QUZzbmcrcnI0Z3RVVU5ydUlWZ09sMlY3dTI4aWdRUnRTZ3AveWlIa0l6eE1E?=
 =?utf-8?B?RDlkQm43RjRNVzdDRkZMSE5ZRXpYSnBDMGlxWTFCUlB3OGp3VDhVNWQwRnhO?=
 =?utf-8?B?Z09mbElMaHloWG9NWkRDbXJHbk84TUJTVHRIVy9KQkd3OW9ubklPMHV5OVNa?=
 =?utf-8?B?Wnc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ca64a7ae-95b9-432a-f28d-08db1a8aec76
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4914.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Mar 2023 19:26:58.5738
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2bPUTeDQhdZr6fYnL2yP2yTFAWTiljJVjK2jT3jDIxi+1sNP8GnvKDl5qZL04V8KP/ao9BlaWEsLWRPSNgrLjZEzZXEmfhkwOEjzAva8v5k=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB7309
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 3/1/2023 9:14 AM, Petr Oros wrote:

>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index b360bd8f15998b..1dc3f9fc74bdfb 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -4293,6 +4293,7 @@ ice_get_module_eeprom(struct net_device *netdev,
>  	bool is_sfp = false;
>  	unsigned int i, j;
>  	u16 offset = 0;
> +	u32 copy_len;

copy_len should only be declared in the "if" below so that it's only
declared in the context it is used.

>  	u8 page = 0;
>  	int status;
>  
> @@ -4354,8 +4355,9 @@ ice_get_module_eeprom(struct net_device *netdev,
>  			}
>  
>  			/* Make sure we have enough room for the new block */
> -			if ((i + SFF_READ_BLOCK_SIZE) < ee->len)
> -				memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
> +			copy_len = min_t(u32, SFF_READ_BLOCK_SIZE,
> +					 ee->len - i);

also this line can be unwrapped now.

> +			memcpy(data + i, value, copy_len);
>  		}
>  	}
>  	return 0;

I've tested this and it fixes the problem. Interestingly, the ethtool
application when compiled without netlink support parsed the output
correctly so caused a bit of confusion around this issue.

Thank you Petr!