On Thu, Mar 19, 2026 at 06:02:12PM -0700, Rosen Penev wrote:
> There's no need to allocate these separately.
> 
> Add __counted_by for extra runtime analysis. Moved counting variable
> allocation to right after allocation as required by __counted_by.
> 
> Signed-off-by: Rosen Penev <[email protected]>

This looks reasonable to me.

One thought I've had while reviewing your flex-array patches is that I
want to make sure you're doing your test builds with
KCFLAGS=-Wflexible-array-member-not-at-end
so that you can validate there's no new uses of the target structures
being composed within other structures while making these changes.

That looks clear here, but I think going forward, it would be worth
mentioning it as part of the commit log. Something like:

  This structure is not composed within other structures, confirmed with
  builds using -Wflexible-array-member-not-at-end.

or similar.

Reviewed-by: Kees Cook <[email protected]>

-Kees

> ---
>  drivers/media/usb/em28xx/em28xx-cards.c | 18 ++----------------
>  drivers/media/usb/em28xx/em28xx.h       |  3 ++-
>  2 files changed, 4 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/usb/em28xx/em28xx-cards.c 
> b/drivers/media/usb/em28xx/em28xx-cards.c
> index d7075ebabceb..c278e48b3428 100644
> --- a/drivers/media/usb/em28xx/em28xx-cards.c
> +++ b/drivers/media/usb/em28xx/em28xx-cards.c
> @@ -3567,9 +3567,6 @@ void em28xx_free_device(struct kref *ref)
>       if (!dev->disconnected)
>               em28xx_release_resources(dev);
>  
> -     if (dev->ts == PRIMARY_TS)
> -             kfree(dev->alt_max_pkt_size_isoc);
> -
>       kfree(dev);
>  }
>  EXPORT_SYMBOL_GPL(em28xx_free_device);
> @@ -3912,21 +3909,13 @@ static int em28xx_usb_probe(struct usb_interface 
> *intf,
>       }
>  
>       /* allocate memory for our device state and initialize it */
> -     dev = kzalloc_obj(*dev);
> +     dev = kzalloc_flex(*dev, alt_max_pkt_size_isoc, intf->num_altsetting);
>       if (!dev) {
>               retval = -ENOMEM;
>               goto err;
>       }
>  
> -     /* compute alternate max packet sizes */
> -     dev->alt_max_pkt_size_isoc = kcalloc(intf->num_altsetting,
> -                                          
> sizeof(dev->alt_max_pkt_size_isoc[0]),
> -                                          GFP_KERNEL);
> -     if (!dev->alt_max_pkt_size_isoc) {
> -             kfree(dev);
> -             retval = -ENOMEM;
> -             goto err;
> -     }
> +     dev->num_alt = intf->num_altsetting;
>  
>       /* Get endpoints */
>       for (i = 0; i < intf->num_altsetting; i++) {
> @@ -4028,8 +4017,6 @@ static int em28xx_usb_probe(struct usb_interface *intf,
>                       dev->dvb_ep_bulk ? " bulk" : "",
>                       dev->dvb_ep_isoc ? " isoc" : "");
>  
> -     dev->num_alt = intf->num_altsetting;
> -
>       if ((unsigned int)card[nr] < em28xx_bcount)
>               dev->model = card[nr];
>  
> @@ -4163,7 +4150,6 @@ static int em28xx_usb_probe(struct usb_interface *intf,
>       return 0;
>  
>  err_free:
> -     kfree(dev->alt_max_pkt_size_isoc);
>       kfree(dev);
>  
>  err:
> diff --git a/drivers/media/usb/em28xx/em28xx.h 
> b/drivers/media/usb/em28xx/em28xx.h
> index f3449c240d21..1c2f92927889 100644
> --- a/drivers/media/usb/em28xx/em28xx.h
> +++ b/drivers/media/usb/em28xx/em28xx.h
> @@ -730,7 +730,6 @@ struct em28xx {
>       int packet_multiplier;  // multiplier for wMaxPacketSize, used for
>                               // URB buffer size definition
>       int num_alt;            // number of alternative settings
> -     unsigned int *alt_max_pkt_size_isoc; // array of isoc wMaxPacketSize
>       unsigned int analog_xfer_bulk:1;        // use bulk instead of isoc
>                                               // transfers for analog
>       int dvb_alt_isoc;       // alternate setting for DVB isoc transfers
> @@ -772,6 +771,8 @@ struct em28xx {
>  
>       struct em28xx   *dev_next;
>       int ts;
> +
> +     unsigned int alt_max_pkt_size_isoc[] __counted_by(num_alt); // array of 
> isoc wMaxPacketSize
>  };
>  
>  #define kref_to_dev(d) container_of(d, struct em28xx, ref)
> -- 
> 2.53.0
> 
> 

-- 
Kees Cook

Reply via email to