Re: [PATCH 1/2] mailbox: add sanity check for channel array

[Jassi Brar]

On Mon, Apr 13, 2026 at 5:42 AM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> Fail gracefully if there is no channel array attached to the mailbox
> controller. Otherwise the later dereference will cause an OOPS which
> might not be seen because mailbox controllers might instantiate very
> early. Remove the comment explaining the obvious while here.
>
> Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox")
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> On my Renesas R-Car X5H based board, the OOPS was indeed silent. I
> injected the error manually.
>
>  drivers/mailbox/mailbox.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> index 14d9655d5424..0ed34f8f7556 100644
> --- a/drivers/mailbox/mailbox.c
> +++ b/drivers/mailbox/mailbox.c
> @@ -533,8 +533,7 @@ int mbox_controller_register(struct mbox_controller *mbox)
>  {
>         int i, txdone;
>
> -       /* Sanity check */
> -       if (!mbox || !mbox->dev || !mbox->ops || !mbox->num_chans)
> +       if (!mbox || !mbox->dev || !mbox->ops || !mbox->chans || !mbox->num_chans)
>                 return -EINVAL;
>
This sounds like a quick suggestion from some AI tool -- looks nice
but doesn't mean much.
Is your controller driver written such that it calls
mbox_controller_register() even if it failed to allocate channels ?
The code already gates access to chans by looking at num_chans. If the
controller sets num_chans to be non-zero, the chans is supposed to
point to an array of channels. NULL is just another garbage value, why
not also check for 0xdeadbabe or 0xffff... ?
I can let loose a claude session and I will find 1000s of such "fixes"
in the kernel.
But since it is here, I am picking it purely as a cosmetic change.
Thanks