Re: [PATCH] mailbox: riscv-sbi-mpxy: Validate notification data length

Anup Patel <[email protected]>
Newsgroups org.infradead.lists.linux-riscv,org.kernel.vger.linux-kernel
Message-ID <CAAhSdy37tQNRfdgkOrQK04XKXcOxmm5Mtrdsi-LvGua+e3rnnQ@mail.gmail.com>
On Sat, Aug 22, 2026 at 11:11 PM liutong <[email protected]> wrote:
>
> mpxy_get_notifications() copies data from shared memory using a
> length derived from the SBI return value:
>
>   memcpy(notif_data, mpxy->shmem, sret.value + 16);
>
> sret.value is not validated against the shared memory buffer size.
> A buggy firmware returning a value larger than mpxy_shmem_size - 16
> would cause memcpy to read beyond the shared memory mapping.

Well, if a firmware is buggy then it can corrupt any part of the
S-mode memory and not just the MPXY shared memory.

I think it is okay to have this additional check but this does
not cover all possible corruptions by buggy firmware.

>
> Add a bounds check before the memcpy.
>
> Fixes: bf3022a4eb11 ("mailbox: Add RISC-V SBI message proxy (MPXY) based mailbox driver")
> Signed-off-by: liutong <[email protected]>

Reviewed-by: Anup Patel <[email protected]>

Regards,
Anup

> ---
>  drivers/mailbox/riscv-sbi-mpxy-mbox.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mailbox/riscv-sbi-mpxy-mbox.c b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> index 18da9efcc..3a2d52bbc 100644
> --- a/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> +++ b/drivers/mailbox/riscv-sbi-mpxy-mbox.c
> @@ -315,6 +315,11 @@ static int mpxy_get_notifications(u32 channel_id,
>         if (sret.error)
>                 goto err_put_cpu;
>
> +       if (sret.value + 16 > mpxy_shmem_size) {
> +               put_cpu();
> +               return -EIO;
> +       }
> +
>         memcpy(notif_data, mpxy->shmem, sret.value + 16);
>         *events_data_len = sret.value;
>
> --
> 2.34.1
>

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.