[PATCH 1/4] staging: vc04_services: vchiq-mmal: fix OOB array access in event_to_host_cb()

[Sebastián Alba]

Hi Greg,

> The kernel trusts the hardware the driver is bound to, so this
> shouldn't be happening ever, right?

You're right that the kernel generally trusts hardware it's bound to.
This is a defensive hardening patch - a malformed index from buggy
firmware would currently cause an uncontrolled OOB access, while with
the check we get a clean error return. Happy to reframe the commit
message as hardening rather than a security fix if you prefer.

> No cc: stable?

Will add Cc: stable at vger.kernel.org in v2.

> dev_err() is best, right?

Agreed, will switch to dev_err() in v2. I'll need to thread the
struct device through - will check how other functions in this
file handle it.

> And are you going to allow a malicious hardware device to spam
> the kernel log?  :)

Good point, will switch to dev_err_ratelimited() in v2.

I'll send a v2 addressing all of these. Thanks for the review.

Sebastián


El dom, 29 mar 2026 a las 0:35, Greg Kroah-Hartman
(<gregkh at linuxfoundation.org>) escribió:
>
> On Sun, Mar 29, 2026 at 12:21:11AM -0600, Sebastian Josue Alba Vives wrote:
> > From: Sebastián Alba Vives <sebasjosue84 at gmail.com>
> >
> > event_to_host_cb() uses msg->u.event_to_host.client_component as an
> > index into the instance->component[] array (size VCHIQ_MMAL_MAX_COMPONENTS
> > = 64) without any bounds validation. The client_component value comes
> > from the VideoCore GPU firmware via VCHIQ message passing.
> >
> > A malicious or buggy GPU firmware could send a crafted
> > MMAL_MSG_TYPE_EVENT_TO_HOST message with client_component >= 64 (or
> > negative), causing an out-of-bounds array access in kernel memory. This
> > results in reading/dereferencing a bogus vchiq_mmal_component structure
> > from memory beyond the array, which can lead to kernel crashes or
> > potentially arbitrary kernel memory access.
>
> The kernel trusts the hardware the driver is bound to, so this shouldn't
> be happening ever, right?
>
> >
> > Add a bounds check on comp_idx before using it as an array index.
> > Move the component pointer assignment after the validation.
> >
> > Fixes: b18ee53ad297 ("staging: bcm2835: Break MMAL support out from camera")
> > Signed-off-by: Sebastián Alba Vives <sebasjosue84 at gmail.com>
>
> No cc: stable?
>
> > ---
> >  drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > index d36ad71cc..4772126d7 100644
> > --- a/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > +++ b/drivers/staging/vc04_services/vchiq-mmal/mmal-vchiq.c
> > @@ -477,12 +477,19 @@ static void event_to_host_cb(struct vchiq_mmal_instance *instance,
> >                            struct mmal_msg *msg, u32 msg_len)
> >  {
> >       int comp_idx = msg->u.event_to_host.client_component;
> > -     struct vchiq_mmal_component *component =
> > -                                     &instance->component[comp_idx];
> > +     struct vchiq_mmal_component *component;
> >       struct vchiq_mmal_port *port = NULL;
> >       struct mmal_msg_context *msg_context;
> >       u32 port_num = msg->u.event_to_host.port_num;
> >
> > +     if (comp_idx < 0 || comp_idx >= VCHIQ_MMAL_MAX_COMPONENTS) {
> > +             pr_err("%s: component index %d out of range\n",
> > +                    __func__, comp_idx);
>
> dev_err() is best, right?
>
> And are you going to allow a malicious hardware device to spam the
> kernel log?  :)
>
> thanks,
>
> greg k-h



-- 
Sebastián Alba