Re: out-of-bounds write in the function ata_pio_sector

[Niklas Cassel <cassel@kernel.org>]

Hello Martin,

On Tue, Jan 28, 2025 at 10:09:35PM -0500, Martin K. Petersen wrote:
> 
> Niklas,
> 
> Sorry about the delay, was out for a few days.
> 
> > I was kind of expecting some upper layer, SCSI or block, to have rejected
> > an operation that is not a multiple of the sector size.
> >
> > Is that a silly assumption?
> 
> Not all SCSI commands operate on logical blocks. Plus even if they do
> the actual data transfer could still be larger than one block due to PI
> or long writes.
> 
> That's all a bit theoretical in the context of the archaic
> sg_scsi_ioctl() call since that only takes a single page and has little
> practical use. But in general we can't assume that everything is a
> multiple of 512 bytes.

Thank you.

I basically came to the same conclusion.
(We can't assume that everything is a multiple of 512 bytes.)


Looking at ACS-6, Table A.2 — Command codes (sorted by command code)
and looking at all commands that are of type:
PIO Data-In and PIO Data-Out.

Most commands use the COUNT field to mean a unit in either sectors or log
page count (which is also a multiple of sectors), but some commands, e.g.
TRUSTED RECEIVE 5Ch and TRUSTED SEND 5Eh, it means TRANSFER LENGTH, which
is security protocol specific.

Looking at TCG (SIIS), TRANSFER LENGTH is a multiple of sectors.
I don't know about other security protocols (if any).


It is probably quite safe to make the assumption that the COUNT field in
ACS will always be a multiple of sectors for PIO Data-In and PIO Data-Out
commands, so we probably could add a check in generic libata code
somewhere... but by adding such a check in generic libata code, for it to
be simple, it would probably need to apply to more than just PIO Data-In
and PIO Data-Out commands, and I'm not sure if we can make such an
assumption.

Thus, I'm happy with the fix:
https://lore.kernel.org/linux-ide/20250127154303.15567-2-cassel@kernel.org/
for now.


Kind regards,
Niklas