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

2020-12-11 Thread leonwxqian

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

2020-12-11 Thread P J P
+-- On Fri, 11 Dec 2020, Paolo Bonzini wrote --+ | 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

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

2020-12-11 Thread Wenxiang Qian
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

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

2020-12-11 Thread Wenxiang Qian
+ 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

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

2020-12-11 Thread Paolo Bonzini
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 >=

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

2020-12-11 Thread 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

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

2020-12-03 Thread P J P
+-- On Wed, 2 Dec 2020, Philippe Mathieu-Daudé wrote --+ | a fair part is to ask the reporter to attach its reproducer to the private | BZ, Yes, reporters sharing/releasing it is best. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050

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

2020-12-02 Thread Philippe Mathieu-Daudé
On 12/2/20 2:17 PM, P J P wrote: > +-- On Tue, 1 Dec 2020, Philippe Mathieu-Daudé wrote --+ > | Is it possible to release the reproducer to the community, so we can work > on > | a fix and test it? > > * No, we can not release/share reproducers on a public list. > > * We can request

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

2020-12-02 Thread Paolo Bonzini
On 02/12/20 14:17, P J P wrote: Hi, [doing a combined reply] +-- On Tue, 1 Dec 2020, Philippe Mathieu-Daudé wrote --+ | Is it possible to release the reproducer to the community, so we can work on | a fix and test it? * No, we can not release/share reproducers on a public list. We do

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

2020-12-02 Thread P J P
Hi, [doing a combined reply] +-- On Tue, 1 Dec 2020, Philippe Mathieu-Daudé wrote --+ | Is it possible to release the reproducer to the community, so we can work on | a fix and test it? * No, we can not release/share reproducers on a public list. * We can request reporters to do so by

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

2020-12-01 Thread Markus Armbruster
Paolo Bonzini writes: > On 01/12/20 16:00, P J P wrote: [...] >> * I did test it against a reproducer, but did not get to the qtest >> part for >>the time constraints. > > qtests are not just helpful. Adding regression tests for bugs is a > *basic* software engineering principle. If you

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

2020-12-01 Thread Paolo Bonzini
On 01/12/20 16:30, Peter Maydell wrote: On Tue, 1 Dec 2020 at 15:28, Philippe Mathieu-Daudé wrote: About reproducer, Michael asked about CVE-2020-24352 (ati_vga OOB in ati_2d_blt) this morning. What happens to reproducers when a CVE is assigned, but the bug is marked as "out of the QEMU

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

2020-12-01 Thread Paolo Bonzini
On 01/12/20 16:23, Philippe Mathieu-Daudé wrote: Hi Prasad, On 12/1/20 4:00 PM, P J P wrote: * I was thinking about checking 'elementary_transfer_size' against 'byte_count_limit', but that did not work out. The loop is confusing there, it first sets elementary_size = size and subtracts

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

2020-12-01 Thread Paolo Bonzini
On 01/12/20 16:00, P J P wrote: * I was thinking about checking 'elementary_transfer_size' against 'byte_count_limit', but that did not work out. The loop is confusing there, it first sets elementary_size = size and subtracts the same Yes, that part is correct. * 's->lba == -1' did not

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

2020-12-01 Thread Peter Maydell
On Tue, 1 Dec 2020 at 15:28, Philippe Mathieu-Daudé wrote: > About reproducer, Michael asked about CVE-2020-24352 (ati_vga OOB in > ati_2d_blt) this morning. What happens to reproducers when a CVE is > assigned, but the bug is marked as "out of the QEMU security boundary"? Also, why are we

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

2020-12-01 Thread Philippe Mathieu-Daudé
Hi Prasad, On 12/1/20 4:00 PM, P J P wrote: > * I was thinking about checking 'elementary_transfer_size' against > 'byte_count_limit', but that did not work out. The loop is confusing there, > it first sets elementary_size = size and subtracts the same If the code is confusing, you can

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

2020-12-01 Thread P J P
Hello Paolo, +-- On Tue, 1 Dec 2020, Paolo Bonzini wrote --+ | 1) Obviously this condition was not in the mind of whoever wrote the code. | For this reason the first thing to understand if how the bug came to happen, | which precondition was not respected. Your backtraces shows that you came

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

2020-12-01 Thread Paolo Bonzini
On 27/11/20 14:57, P J P wrote: +-- On Wed, 18 Nov 2020, P J P wrote --+ | During data transfer via packet command in 'ide_atapi_cmd_reply_end' | 's->io_buffer_index' could exceed the 's->io_buffer' length, leading | to OOB access issue. Add check to avoid it. | ... | #9 ahci_pio_transfer

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

2020-11-27 Thread P J P
+-- On Wed, 18 Nov 2020, P J P wrote --+ | During data transfer via packet command in 'ide_atapi_cmd_reply_end' | 's->io_buffer_index' could exceed the 's->io_buffer' length, leading | to OOB access issue. Add check to avoid it. | ... | #9 ahci_pio_transfer ../hw/ide/ahci.c:1383 | #10

[PATCH] ide:atapi: check io_buffer_index in ide_atapi_cmd_reply_end

2020-11-18 Thread P J P
From: Prasad J Pandit During data transfer via packet command in 'ide_atapi_cmd_reply_end' 's->io_buffer_index' could exceed the 's->io_buffer' length, leading to OOB access issue. Add check to avoid it. ... #9 ahci_pio_transfer ../hw/ide/ahci.c:1383 #10 ide_transfer_start_norecurse