Re: [PATCH v2] ata: libata-scsi: limit simulated SCSI command copy to response length

On Wed, Jul 1, 2026 at 7:30 PM Damien Le Moal <[email protected]> wrote:
>
> On 7/2/26 11:14, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> >
> > Pre-existing issues:
> > - [Critical] The defensive bounds check `WARN_ON(len > ATA_SCSI_RBUF_SIZE)` is ineffective because it evaluates after the `actor` callback has already performed an out-of-bounds write.
>
> Karuna,
>
> Sashiko has a good point here. Let's work on fixing this with an incremental patch.
>
>
> --
> Damien Le Moal
> Western Digital Research

Hi Damien,

Thanks for reviewing and applying the fix patch!

I reviewed the actor functions to evaluate how to structure this
incremental patch fix.
Since the actors write to the static buffer `ata_scsi_rbuf` defined in
`libata-scsi.c` via the `rbuf` input pointer, the most robust way to
enforce bounds would be to update the actor callback signature to
accept `size_t rbuf_len` along with the `u8 *rbuf` pointer.

All the simulation actor functions (like `ata_scsiop_inq_std`,
`ata_scsiop_inq_b9`, etc) are declared static inside `libata-scsi.c`
so this change would be entirely self-contained. Each actor could
then perform bounds checks at the point of writing and return 0
(eventually failing with ABORTED_COMMAND) before an overflow
occurs.

However this refactoring will touch ~15 functions starting from the
actor callback, until all the helper functions, and result in a
~100-200 line patch.

Before I start working on this, I would love to hear your feedback on:
1. Whether you think this API update (passing `rbuf_len` down to all
actors and helper functions) is the right approach, or if you prefer
simpler, and more localized checks elsewhere.
2. What would be the best way to test all these simulated command
paths to ensure the fix is verified appropriately?

Thank you,
Karuna