[PATCH net] pds_core: limit loop over fw name list

[PATCH net] pds_core: limit loop over fw name list

[Shannon Nelson]

Add an array size limit to the for-loop to be sure we don't try
to reference a fw_version string off the end of the fw info names
array.  We know that our firmware only has a limited number
of firmware slot names, but we shouldn't leave this unchecked.

Fixes: 45d76f492938 ("pds_core: set up device and adminq")
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
 drivers/net/ethernet/amd/pds_core/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index 2681889162a2..44971e71991f 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -118,7 +118,7 @@ int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
 	if (err && err != -EIO)
 		return err;
 
-	listlen = fw_list.num_fw_slots;
+	listlen = min(fw_list.num_fw_slots, ARRAY_SIZE(fw_list.fw_names));
 	for (i = 0; i < listlen; i++) {
 		if (i < ARRAY_SIZE(fw_slotnames))
 			strscpy(buf, fw_slotnames[i], sizeof(buf));
-- 
2.17.1

Re: [PATCH net] pds_core: limit loop over fw name list

[Simon Horman]

On Fri, Jan 03, 2025 at 11:51:47AM -0800, Shannon Nelson wrote:
> Add an array size limit to the for-loop to be sure we don't try
> to reference a fw_version string off the end of the fw info names
> array.  We know that our firmware only has a limited number
> of firmware slot names, but we shouldn't leave this unchecked.
> 
> Fixes: 45d76f492938 ("pds_core: set up device and adminq")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net] pds_core: limit loop over fw name list

[Brett Creeley]

On 1/3/2025 11:51 AM, Shannon Nelson wrote:
> Add an array size limit to the for-loop to be sure we don't try
> to reference a fw_version string off the end of the fw info names
> array.  We know that our firmware only has a limited number
> of firmware slot names, but we shouldn't leave this unchecked.
> 
> Fixes: 45d76f492938 ("pds_core: set up device and adminq")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
>   drivers/net/ethernet/amd/pds_core/devlink.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
> index 2681889162a2..44971e71991f 100644
> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
> @@ -118,7 +118,7 @@ int pdsc_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
>   	if (err && err != -EIO)
>   		return err;
>   
> -	listlen = fw_list.num_fw_slots;
> +	listlen = min(fw_list.num_fw_slots, ARRAY_SIZE(fw_list.fw_names));

LGTM. Thanks!

Reviewed-by: Brett Creeley <brett.creeley@amd.com>

>   	for (i = 0; i < listlen; i++) {
>   		if (i < ARRAY_SIZE(fw_slotnames))
>   			strscpy(buf, fw_slotnames[i], sizeof(buf));

RE: [PATCH net] pds_core: limit loop over fw name list

[Keller, Jacob E]

> -----Original Message-----
> From: Shannon Nelson <shannon.nelson@amd.com>
> Sent: Friday, January 3, 2025 11:52 AM
> To: netdev@vger.kernel.org; davem@davemloft.net; kuba@kernel.org;
> edumazet@google.com; pabeni@redhat.com; andrew+netdev@lunn.ch; Keller,
> Jacob E <jacob.e.keller@intel.com>
> Cc: brett.creeley@amd.com; Shannon Nelson <shannon.nelson@amd.com>
> Subject: [PATCH net] pds_core: limit loop over fw name list
> 
> Add an array size limit to the for-loop to be sure we don't try
> to reference a fw_version string off the end of the fw info names
> array.  We know that our firmware only has a limited number
> of firmware slot names, but we shouldn't leave this unchecked.
> 
> Fixes: 45d76f492938 ("pds_core: set up device and adminq")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
>  drivers/net/ethernet/amd/pds_core/devlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c
> b/drivers/net/ethernet/amd/pds_core/devlink.c
> index 2681889162a2..44971e71991f 100644
> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
> @@ -118,7 +118,7 @@ int pdsc_dl_info_get(struct devlink *dl, struct
> devlink_info_req *req,
>  	if (err && err != -EIO)
>  		return err;
> 
> -	listlen = fw_list.num_fw_slots;
> +	listlen = min(fw_list.num_fw_slots, ARRAY_SIZE(fw_list.fw_names));

I assume num_fw_slots comes from the firmware? Makes sense to ensure we limit in case somehow the value no longer matches in the future.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

Re: [PATCH net] pds_core: limit loop over fw name list

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 3 Jan 2025 11:51:47 -0800 you wrote:
> Add an array size limit to the for-loop to be sure we don't try
> to reference a fw_version string off the end of the fw info names
> array.  We know that our firmware only has a limited number
> of firmware slot names, but we shouldn't leave this unchecked.
> 
> Fixes: 45d76f492938 ("pds_core: set up device and adminq")
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> 
> [...]

Here is the summary with links:
  - [net] pds_core: limit loop over fw name list
    https://git.kernel.org/netdev/net/c/8c817eb26230

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html