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

[leonwxqian]

Hi Paolo,

Yes, you are right. I read the logic of the code again, If lba=-1 is restricted, this path will not be triggered.And I think that your fix is the best solution to this issue.

Best regards,

Wenxiang Qian

---Original---

From: "Paolo Bonzini"<pbonzini@redhat.com>

Date: Fri, Dec 11, 2020 19:46 PM

To: "Wenxiang Qian"<leonwxqian@gmail.com>;"P J P"<ppandit@redhat.com>;

Cc: "John Snow"<jsnow@redhat.com>;"QEMU Developers"<qemu-devel@nongnu.org>;"Markus Armbruster"<armbru@redhat.com>;"Peter Maydell"<peter.maydell@linaro.org>;"Philippe Mathieu-Daudé"<philmd@redhat.com>;

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

On 11/12/20 09:32, Wenxiang Qian wrote:
> + The lba is set to -1 to avoid some code paths, to make PoC simpler.
> 
> void ide_atapi_cmd_reply_end(IDEState *s)
> {
>      int byte_count_limit, size, ret;
>      while (s->packet_transfer_size > 0) {
> .....
>          if (s->lba != -1 && s->io_buffer_index >= s->cd_sector_size) { 
> <----- set lba to -1 to avoid this part
>   .....
>          }
>          if (s->elementary_transfer_size > 0) {
> ......
>          } else {
> .......
>              if (s->lba != -1) { <-----
>                  if (size > (s->cd_sector_size - s->io_buffer_index))
>                      size = (s->cd_sector_size - s->io_buffer_index);  
> <-----
>              }
>          }
> 

If the lba is not -1, I don't think bad things can happen on this path. 
  Am I wrong?

Paolo

)
* 1254 EXPUNGE
* 1253 EXIST