Re: [PATCHv2] 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 calculates an incorrect consumed byte count due to the `words` loop counter underflowing, returning either `0xFFFFFFFE` or `0`.
--

--- Patch [1]: [PATCHv2] ata: pata_ep93xx: fix endianness bug in trailing byte transfer ---
commit 6f058310f6ea9721e68fb8c4613fa6b146f2e1f7
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: fix endianness bug in trailing byte transfer

This commit fixes an endianness issue in the trailing odd byte transfer
logic by removing the pad array and directly reading/writing the u8
value without wrappers.

Closes: https://lore.kernel.org/oe-kbuild-all/202607231521.gvhnJpOG-lkp@intel.com/

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index 42a24dc51d26b..647977ed607c8 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -499,19 +499,12 @@ static unsigned int ep93xx_pata_data_xfer(struct ata_queued_cmd *qc,
>  
>  	/* Transfer trailing 1 byte, if any. */
>  	if (unlikely(buflen & 0x01)) {
> -		unsigned char pad[2] = { };
> -
>  		buf += buflen - 1;
>  
>  		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 calculate the correct
consumed byte count?

When ep93xx_pata_data_xfer() executes the earlier transfer loop, the words
variable unconditionally decrements until it underflows:

drivers/ata/pata_ep93xx.c:ep93xx_pata_data_xfer() {
    ...
    unsigned int words = buflen >> 1;

    /* Transfer multiple of 2 bytes */
    while (words--)
        if (rw == READ)
            ...
}

After the loop, words underflows to UINT_MAX.

For odd-length transfers, the trailing byte block quoted above executes
words++, which increments UINT_MAX to 0. The function then returns words << 1,
which results in returning 0.

If 0 is returned, could this cause an infinite loop in __atapi_pio_bytes()? A
returned consumed value of 0 means qc->curbytes does not advance, which might
cause the transfer loop to stall indefinitely.

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