Re: [PATCH] ata: pata_ep93xx: fix endianness bug in trailing byte transfer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `ep93xx_pata_data_xfer()` function destroys the `words` loop counter, returning an incorrect byte count that breaks ATAPI PIO transfers.
--

commit c92ab9f0ecc44ddedc25d0e57302962565e67769
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: fix endianness bug in trailing byte transfer

This commit fixes an endianness bug in ep93xx_pata_data_xfer() where a trailing
odd byte was being truncated and incorrectly byte-swapped on big-endian
systems. It removes the cpu_to_le16/le16_to_cpu wrappers and uses the raw
read/write values directly.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -495,19 +495,12 @@ static unsigned int ep93xx_pata_data_xfer(struct ata_queued_cmd *qc,
>  
>  	/* Transfer trailing 1 byte, if any. */
>  	if (unlikely(buflen & 0x01)) {

[ ... ]

>  		if (rw == READ) {
> -			*pad = cpu_to_le16(
> -				ep93xx_pata_read_data(
> -					drv_data, IDECTRL_ADDR_DATA));
> -			*buf = pad[0];
> +			*buf = ep93xx_pata_read_data(drv_data, IDECTRL_ADDR_DATA);
>  		} else {
> -			pad[0] = *buf;
> -			ep93xx_pata_write_data(drv_data, le16_to_cpu(*pad),
> -					  IDECTRL_ADDR_DATA);
> +			ep93xx_pata_write_data(drv_data, *buf, IDECTRL_ADDR_DATA);
>  		}
>  		words++;

[Severity: High]
This is a pre-existing issue, but does this function return an incorrect byte
count that breaks ATAPI PIO transfers?

Earlier in ep93xx_pata_data_xfer(), there is a loop that destroys the words
counter:

        unsigned int words = buflen >> 1;
        while (words--)
            /* transfer 2 bytes */

Because of the post-decrement, the words counter underflows to UINT_MAX when
the loop terminates. If there is an odd trailing byte, words++ wraps it back
to 0, and the function returns 0 since the end of the function is:

        return words << 1;

If there is no trailing byte, the function returns UINT_MAX << 1, which is
0xFFFFFFFE.

Returning 0 for an odd-length transfer causes callers like
__atapi_pio_bytes() to not decrement their remaining bytes counter while
still advancing the scatter-gather list, resulting in unexpected or too
much trailing data errors.

Returning 0xFFFFFFFE causes __atapi_pio_bytes() to subtract that huge amount
in a single iteration, instantly dropping the remaining transfer count to zero
even if the DRQ block spanned multiple scatter-gather entries.

Could this be fixed by saving the original buffer length and returning that
instead of relying on the mutated words counter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601190921.20167-1-rosenp@gmail.com?part=1