Re: [PATCH] ata: libata-eh: Fix detection of deferred qc timeouts

[Niklas Cassel]

On Fri, Mar 06, 2026 at 12:33:21PM +0900, Damien Le Moal wrote:
> On 3/6/26 11:48, Guenter Roeck wrote:
> > If the `ata_qc_for_each_raw()` loop finishes without finding a matching
> > `scmd`, `qc` will hold a pointer to the last element examined, which is
> > accociated with `i == ATA_MAX_QUEUE - 1`. This element can match
> > `ap->deferred_qc`. If that happens, the condition `qc == ap->deferred_qc`
> > evaluates to true despite the loop not breaking on a scmd match.
> > In that case, the error handler mistakenly intercepts a command that
> > completed normally after an unrelated SCSI timeout, returning a timeout
> > error instead of success.
> > 
> > Fix the problem by checking for i < ATA_MAX_QUEUE in addition to
> > qc == ap->deferred_qc.
> > 
> > The problem was found by an experimental code review agent based on
> > gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> Please remove all the quotes:
> 
> If the ata_qc_for_each_raw() loop finishes without finding a matching SCSI
> command for any QC, the variable qc will hold a pointer to the last element
> examined, which has the tag i == ATA_MAX_QUEUE - 1. This qc can match the port
> deferred QC (ap->deferred_qc).
> 
> If that happens, the condition qc == ap->deferred_qc evaluates to true despite
> the loop not breaking with a match on the SCSI command for this QC. In that
> case, the error handler mistakenly intercepts a command that not completed yet

Could even write:
"that has not been issued yet"

(while it is true that it has not been completed yet, it has not even been
issued to the drive yet.)


> and that has not timed out, and thus erroneously returning a timeout error.
> 
> Fix the problem by checking for i < ATA_MAX_QUEUE in addition to
> qc == ap->deferred_qc.
> 
> The problem was found by an experimental code review agent based on
> gemini-3.1-pro while reviewing backports into v6.18.y.
> 
> > Assisted-by: Gemini:gemini-3.1-pro
> > Cc: Damien Le Moal <dlemoal@xxxxxxxxxx>
> > Cc: Niklas Cassel <cassel@xxxxxxxxxx>
> > Fixes: eddb98ad9364 ("ata: libata-eh: correctly handle deferred qc timeouts")
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> 
> With the above, feel free to add:
> 
> Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> 
> Thanks !
> 
> -- 
> Damien Le Moal
> Western Digital Research