Re: [PATCH 06/17] mpi3mr: Fix out-of-bounds reply frame access

On Tue, Aug 18, 2026 at 11:41 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Low] The newly added modulo operation `((phys_addr - mrioc->reply_buf_dma) % mrioc->reply_sz)` implicitly performs a 64-bit division, causing a build failure (linker error) on 32-bit platforms where `dma_addr_t` is 64-bit.
>
> Pre-existing issues:
> - [High] An integer overflow in the reply pool size calculation allows a malicious or compromised device to trigger a massive out-of-bounds DMA write, leading to host memory corruption.
> --
>
> --- Patch [6]: [PATCH 06/17] mpi3mr: Fix out-of-bounds reply frame access ---
> commit 144f6c8866bb0bb9ede9699c897280faf1028d42
> Author: Chandrakanth Patil <[email protected]>
>
> mpi3mr: Fix out-of-bounds reply frame access
>
> This commit tightens the validation of reply frame addresses by requiring
> the address to be frame-aligned and ensuring a whole frame fits within the
> pool.
>
> > diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > index 681868716ebdb..5e61448d8dbc6 100644
> > --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> > @@ -128,7 +128,8 @@ void *mpi3mr_get_reply_virt_addr(struct mpi3mr_ioc *mrioc,
> >               return NULL;
> >
> >       if ((phys_addr < mrioc->reply_buf_dma) ||
> > -         (phys_addr > mrioc->reply_buf_dma_max_address))
> > +         (phys_addr > mrioc->reply_buf_dma_max_address - mrioc->reply_sz) ||
> > +         ((phys_addr - mrioc->reply_buf_dma) % mrioc->reply_sz))
>
> [Severity: Low]
> Will this modulo operation cause a linker error on 32-bit platforms?
>
> Since dma_addr_t can be 64-bit on 32-bit architectures (like x86 PAE), using
> the modulo operator on it might implicitly perform a 64-bit division. This
> typically results in an undefined reference to __umoddi3 during the build.
>
> Could this be avoided by using explicit division macros like div64_u64_rem()
> or similar?
>
> >               return NULL;
> >
> >       return mrioc->reply_buf + (phys_addr - mrioc->reply_buf_dma);
>
> [Severity: High]
> This is a pre-existing issue, but does the reply pool size calculation in
> mpi3mr_alloc_reply_sense_bufs() suffer from a 32-bit integer overflow?
>
> If a device provides crafted values for maximum outstanding requests and
> reply frame size, this multiplication might overflow:
>
> drivers/scsi/mpi3mr/mpi3mr_fw.c:mpi3mr_alloc_reply_sense_bufs() {
>     ...
>     u32 sz;
>     ...
>     sz = mrioc->num_reply_bufs * mrioc->reply_sz;
>     ...
> }
>
> If this overflows, an undersized DMA pool would be created. Later, in
> mpimr_initialize_reply_sbuf_queues(), the driver loops over the expected
> number of buffers and writes out-of-bounds DMA addresses to the hardware:
>
> drivers/scsi/mpi3mr/mpi3mr_fw.c:mpimr_initialize_reply_sbuf_queues() {
>     ...
>     for (i = 0, phy_addr = mrioc->reply_buf_dma;
>          i < mrioc->num_reply_bufs; i++, phy_addr += mrioc->reply_sz)
>         mrioc->reply_free_q[i] = cpu_to_le64(phy_addr);
>     ...
> }
>
> Could this allow a malicious or compromised device to trigger out-of-bounds
> DMA writes into host memory?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

Thanks for the review.

New issues:
- The 32-bit build issue is valid and will be fixed in the v2
submission. switching to do_div() for alignment checking to avoid
64-bit division issues on 32-bit platforms.

Pre-existing issues:
- The reply pool size calculation overflow is a pre-existing issue in
the driver and will be addressed in a separate follow-up patch series.