Re: [PATCH] ide:atapi: check io_buffer_index in ide_atapi_cmd_reply_end

[Paolo Bonzini]

On 11/12/20 09:23, Wenxiang Qian wrote:
> Hello,
>
> I may not have made the detail clear in my previous email. The details 
> of the AHCI device, after running the reproducer I attached in my report 
> are as follows. If there is any information I can provide, please let me 
> know. Thank you.
>
> ###root cause###
> (1) The s->packet_transfer_size is bigger than the actual data.
> (2) packet_transfer_size is passed into  ide_atapi_cmd_reply_end, as the 
> total number of iterations. Each iterate round, s->io_buffer_index is 
> increased by 2048, but without boundary check.
> (3) The call to ide_transfer_start_norecurse use s->io_buffer + 
> s->io_buffer_index - size as the index, cause an OOB access.

This is not the root cause.  These are the last steps before bad things 
happen; the root cause is what _led_ to those last steps.  In this case, 
the root cause is that a read request with s->lba == -1 is mistaken for 
a non-read.  Read requests are able to reset s->io_buffer_index and 
start with the index pointing just after the end of the sector buffer; 
non-read requests instead visit the buffer just once and start with 
s->io_buffer_index == 0.

In turn, the fix is to validate:

1) that s->lba is in range when issuing a read request

2) that the size of the device is sane (e.g. the number of blocks is a 
positive 32-bit integer).

Paolo