| Message ID | 20170606121747.25356-3-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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). 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); >
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); >>
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);
Avoid TOC-TOU bugs depending on how the compiler behaves. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/scsi/megasas.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)