Re: [PATCH net v3 3/3] net: fealnx: bound card_idx to prevent out-of-bounds access

[Fedor Pchelkin <pchelkin@ispras.ru>]

On Mon, 19. Oct 13:43, Жамбакиев Радий Рикардинович wrote:
> diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
> index b5e96c7037f3..d5931c0314fc 100644
> --- a/drivers/net/ethernet/fealnx.c
> +++ b/drivers/net/ethernet/fealnx.c
> @@ -41,6 +41,7 @@ static int rx_copybreak;
>  /* interoperability.                                            */
>  /* The media type is usually passed in 'options[]'.             */
>  #define MAX_UNITS 8		/* More are supported, limit only on options */
> +#define MAX_CARD_IDX		99999
>  static int options[MAX_UNITS] = { -1, -1, -1, -1, -1, -1, -1, -1 };
>  static int full_duplex[MAX_UNITS] = { -1, -1, -1, -1, -1, -1, -1, -1 };
>  
> @@ -489,6 +490,10 @@ static int fealnx_init_one(struct pci_dev *pdev,
>  	int bar = 1;
>  #endif
>  
> +	if (card_idx >= MAX_CARD_IDX) {
> +		dev_err(&pdev->dev, "card_idx is too big %d, aborting", card_idx);
> +		return -ENODEV;
> +	}
>  	card_idx++;
>  	sprintf(boardname, "fealnx%d", card_idx);

There is a compiler warning with a W=1 build the testing bots will
soon complain about:

drivers/net/ethernet/fealnx.c:498:35: warning: '%d' directive writing between 1 and 11 bytes into a region of size 6 [-Wformat-overflow=]
  498 |         sprintf(boardname, "fealnx%d", card_idx);
      |                                   ^~
drivers/net/ethernet/fealnx.c:498:28: note: directive argument in the range [-2147483647, 99999]
  498 |         sprintf(boardname, "fealnx%d", card_idx);
      |                            ^~~~~~~~~~
drivers/net/ethernet/fealnx.c:498:9: note: 'sprintf' output between 8 and 18 bytes into a destination of size 12
  498 |         sprintf(boardname, "fealnx%d", card_idx);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~