[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1] qemu-pr-helper: garbage response structure c
Re: [Qemu-devel] [PATCH v1] qemu-pr-helper: garbage response structure can be used to write data
Tue, 3 Jul 2018 11:35:49 +0200
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0
On 03/07/2018 11:27, Dima Stepanov wrote:
> On Mon, Jul 02, 2018 at 02:21:41PM +0200, Paolo Bonzini wrote:
>> On 02/07/2018 10:52, Dima Stepanov wrote:
>>> On Fri, Jun 15, 2018 at 12:11:44PM +0300, Dima Stepanov wrote:
>>>> The prh_co_entry() routine handles requests. The first part is to read a
>>>> request by calling the prh_read_request() routine, if:
>>>> 1. scsi_cdb_xfer(req->cdb) call returns 0, and
>>>> 2. req->cdb == PERSISTENT_RESERVE_IN, then
>>>> The resp->result field will be uninitialized. As a result the resp.sz
>>>> field will be also uninitialized in the prh_co_entry() function.
>>>> The second part is to send the response by calling the
>>>> prh_write_response() routine:
>>>> 1. For the PERSISTENT_RESERVE_IN command, and
>>>> 2. resp->result == GOOD (previous successful reply or just luck), then
>>>> There is a probability that the following assert will not be trigered:
>>>> assert(resp->sz <= req->sz && resp->sz <= sizeof(client->data));
>>>> As a result some uninitialized response will be sent.
>>>> The fix is to initialize the response structure to CHECK_CONDITION and 0
>>>> values before calling the prh_read_request() routine.
>> The actual bug is that the "if (sz > 0)" should apply only to
>> PERSISTENT_RESERVE_OUT, and in fact it can be done in do_pr_out.
>> PERSISTENT_RESERVE_IN with sz == 0 is weird but okay.
>> This simplifies the code a bit too, because we can handle closing the
>> file descriptor in prh_co_entry.
>> Does something like this work for you?
> Thanks for the feedback. Yes, this will work for me. Should i update the
> patch and resend it or you will just pick the version you suggested?
I will pick it, thanks!