Re: [PATCH] firmware: arm_scmi: simplify some allocations
Sudeep Holla <[email protected]> Tue, 30 Jun 2026 10:01:01 +0100
| Newsgroups | org.kernel.vger.arm-scmi |
|---|---|
| Message-ID | <20260630-pillbug-of-major-brotherhood-ca55f7@sudeepholla> |
On Sun, Jun 07, 2026 at 10:07:24PM -0700, Rosen Penev wrote: > Use flexible array members to combine allocations and remove kcalloc > usage. > > Add __counted_by where appropriate for extra runtime analysis. Move > counting variable assignment after allocation before any potential array > access. > > Signed-off-by: Rosen Penev <[email protected]> > --- > drivers/firmware/arm_scmi/notify.c | 42 ++++++++++++------------------ > 1 file changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c > index 40ec184eedae..45cbb3d2d684 100644 > --- a/drivers/firmware/arm_scmi/notify.c > +++ b/drivers/firmware/arm_scmi/notify.c [...] > @@ -722,12 +725,6 @@ scmi_allocate_registered_events_desc(struct scmi_notify_instance *ni, > return ERR_PTR(-ENOMEM); > pd->eh_sz = eh_sz; > > - pd->registered_events = devm_kcalloc(ni->handle->dev, num_events, > - sizeof(char *), GFP_KERNEL); > - if (!pd->registered_events) > - return ERR_PTR(-ENOMEM); > - pd->num_events = num_events; > - > /* Initialize per protocol handlers table */ > mutex_init(&pd->registered_mtx); > hash_init(pd->registered_events_handlers); > @@ -800,14 +797,11 @@ int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id, > GFP_KERNEL); > if (!r_evt) > return -ENOMEM; Look at Sashiko's report[1]. | Does this allocation leave enough room for the flexible array? | Now that sources has been converted to a flexible array member in | struct scmi_registered_event, sizeof(*r_evt) will not include space | for it. Should this be updated to use struct_size(r_evt, sources, | num_sources)? | Without allocating space for the flexible array, the later call to | refcount_set(&r_evt->sources[id], NOTIF_UNSUPP) in this function | will result in an out-of-bounds heap write. I think you need the below diff as well. Regards, Sudeep [1] https://sashiko.dev/#/patchset/[email protected] -->8 diff --git i/drivers/firmware/arm_scmi/notify.c w/drivers/firmware/arm_scmi/notify.c index 45cbb3d2d684..3abf7a28ff7e 100644 --- i/drivers/firmware/arm_scmi/notify.c +++ w/drivers/firmware/arm_scmi/notify.c @@ -793,7 +793,8 @@ int scmi_register_protocol_events(const struct scmi_handle *handle, u8 proto_id, int id; struct scmi_registered_event *r_evt; - r_evt = devm_kzalloc(ni->handle->dev, sizeof(*r_evt), + r_evt = devm_kzalloc(ni->handle->dev, + struct_size(r_evt, sources, num_sources), GFP_KERNEL); if (!r_evt) return -ENOMEM;