Re: [Qemu-devel] [PATCH 2/7] megasas: do not read sense length more than once from frame

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/06/2017 15:26, Philippe Mathieu-Daudé wrote:
> Hi Paolo,
> 
> Should this patch go in qemu-stable?
> 
> On 06/06/2017 09:17 AM, Paolo Bonzini wrote:
>> Avoid TOC-TOU bugs depending on how the compiler behaves.
> 
> Can you be more descriptive here? Which compiler? (thinking about how to
> prevent this class of bugs).

If you do

int f(int *x)
{
   if (*x == 0) {
       return 42;
   }
   return *x;
}

It's possible that the guest races against the two reads and f() returns
zero.  This is unlikely in the case at least at -O2, but pretty likely
at -O0.

For other patches later in the series, there is no compiler dependence
at all, since the two accesses occur very far from each other.

There is no way really to prevent them except being careful: the rule is
basically that guest-provided data must only be read once.

Thanks,

Paolo

> Regards,
> 
> Phil.
> 
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/scsi/megasas.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
>> index 804122ab05..1888118e5f 100644
>> --- a/hw/scsi/megasas.c
>> +++ b/hw/scsi/megasas.c
>> @@ -309,9 +309,11 @@ static int megasas_build_sense(MegasasCmd *cmd,
>> uint8_t *sense_ptr,
>>      PCIDevice *pcid = PCI_DEVICE(cmd->state);
>>      uint32_t pa_hi = 0, pa_lo;
>>      hwaddr pa;
>> +    int frame_sense_len;
>>
>> -    if (sense_len > cmd->frame->header.sense_len) {
>> -        sense_len = cmd->frame->header.sense_len;
>> +    frame_sense_len = cmd->frame->header.sense_len;
>> +    if (sense_len > frame_sense_len) {
>> +        sense_len = frame_sense_len;
>>      }
>>      if (sense_len) {
>>          pa_lo = le32_to_cpu(cmd->frame->pass.sense_addr_lo);
>>