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;