* [PATCH] media: em28xx: kzalloc + kcalloc to kzalloc_flex
@ 2026-03-20 1:02 Rosen Penev
2026-03-20 18:42 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Rosen Penev @ 2026-03-20 1:02 UTC (permalink / raw)
To: linux-media
Cc: Mauro Carvalho Chehab, Kees Cook, Gustavo A. R. Silva, open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
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 <rosenp@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] media: em28xx: kzalloc + kcalloc to kzalloc_flex
2026-03-20 1:02 [PATCH] media: em28xx: kzalloc + kcalloc to kzalloc_flex Rosen Penev
@ 2026-03-20 18:42 ` Kees Cook
2026-03-20 23:04 ` Rosen Penev
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2026-03-20 18:42 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-media, Mauro Carvalho Chehab, Gustavo A. R. Silva,
open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
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 <rosenp@gmail.com>
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 <kees@kernel.org>
-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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] media: em28xx: kzalloc + kcalloc to kzalloc_flex
2026-03-20 18:42 ` Kees Cook
@ 2026-03-20 23:04 ` Rosen Penev
0 siblings, 0 replies; 3+ messages in thread
From: Rosen Penev @ 2026-03-20 23:04 UTC (permalink / raw)
To: Kees Cook
Cc: linux-media, Mauro Carvalho Chehab, Gustavo A. R. Silva,
open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
On Fri, Mar 20, 2026 at 11:42 AM Kees Cook <kees@kernel.org> wrote:
>
> 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 <rosenp@gmail.com>
>
> 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.
I did that globally. Too many pre-existing issues with that.
I mostly just run make menuconfig on linux-next and see what happens.
>
> 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 <kees@kernel.org>
>
> -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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-20 23:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 1:02 [PATCH] media: em28xx: kzalloc + kcalloc to kzalloc_flex Rosen Penev
2026-03-20 18:42 ` Kees Cook
2026-03-20 23:04 ` Rosen Penev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox