[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 37/38] scsi/scsi_bus: fix races in REPORT LUNS
From: |
Paolo Bonzini |
Subject: |
[PULL 37/38] scsi/scsi_bus: fix races in REPORT LUNS |
Date: |
Mon, 12 Oct 2020 16:33:42 -0400 |
From: Maxim Levitsky <mlevitsk@redhat.com>
Currently scsi_target_emulate_report_luns iterates over the child device list
twice, and there is no guarantee that this list is the same in both iterations.
The reason for iterating twice is that the first iteration calculates
how much memory to allocate. However if we use a dynamic array we can
avoid iterating twice, and therefore we avoid this race.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1866707
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-10-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-14-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-bus.c | 68 ++++++++++++++++++++++------------------------
1 file changed, 33 insertions(+), 35 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index eda8cb7e70..b901e701f0 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -438,19 +438,23 @@ struct SCSITargetReq {
static void store_lun(uint8_t *outbuf, int lun)
{
if (lun < 256) {
+ /* Simple logical unit addressing method*/
+ outbuf[0] = 0;
outbuf[1] = lun;
- return;
+ } else {
+ /* Flat space addressing method */
+ outbuf[0] = 0x40 | (lun >> 8);
+ outbuf[1] = (lun & 255);
}
- outbuf[1] = (lun & 255);
- outbuf[0] = (lun >> 8) | 0x40;
}
static bool scsi_target_emulate_report_luns(SCSITargetReq *r)
{
BusChild *kid;
- int i, len, n;
int channel, id;
- bool found_lun0;
+ uint8_t tmp[8] = {0};
+ int len = 0;
+ GByteArray *buf;
if (r->req.cmd.xfer < 16) {
return false;
@@ -458,46 +462,40 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq
*r)
if (r->req.cmd.buf[2] > 2) {
return false;
}
+
+ /* reserve space for 63 LUNs*/
+ buf = g_byte_array_sized_new(512);
+
channel = r->req.dev->channel;
id = r->req.dev->id;
- found_lun0 = false;
- n = 0;
- RCU_READ_LOCK_GUARD();
+ /* add size (will be updated later to correct value */
+ g_byte_array_append(buf, tmp, 8);
+ len += 8;
- QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
- DeviceState *qdev = kid->child;
- SCSIDevice *dev = SCSI_DEVICE(qdev);
+ /* add LUN0 */
+ g_byte_array_append(buf, tmp, 8);
+ len += 8;
- if (dev->channel == channel && dev->id == id) {
- if (dev->lun == 0) {
- found_lun0 = true;
+ WITH_RCU_READ_LOCK_GUARD() {
+ QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
+ DeviceState *qdev = kid->child;
+ SCSIDevice *dev = SCSI_DEVICE(qdev);
+
+ if (dev->channel == channel && dev->id == id && dev->lun != 0) {
+ store_lun(tmp, dev->lun);
+ g_byte_array_append(buf, tmp, 8);
+ len += 8;
}
- n += 8;
}
}
- if (!found_lun0) {
- n += 8;
- }
-
- scsi_target_alloc_buf(&r->req, n + 8);
-
- len = MIN(n + 8, r->req.cmd.xfer & ~7);
- memset(r->buf, 0, len);
- stl_be_p(&r->buf[0], n);
- i = found_lun0 ? 8 : 16;
- QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) {
- DeviceState *qdev = kid->child;
- SCSIDevice *dev = SCSI_DEVICE(qdev);
- if (dev->channel == channel && dev->id == id) {
- store_lun(&r->buf[i], dev->lun);
- i += 8;
- }
- }
+ r->buf_len = len;
+ r->buf = g_byte_array_free(buf, FALSE);
+ r->len = MIN(len, r->req.cmd.xfer & ~7);
- assert(i == n + 8);
- r->len = len;
+ /* store the LUN list length */
+ stl_be_p(&r->buf[0], len - 8);
return true;
}
--
2.26.2
- [PULL 20/38] meson.build: Re-enable KVM support for MIPS, (continued)
- [PULL 20/38] meson.build: Re-enable KVM support for MIPS, Paolo Bonzini, 2020/10/12
- [PULL 21/38] qtest: rename qtest_qmp_receive to qtest_qmp_receive_dict, Paolo Bonzini, 2020/10/12
- [PULL 14/38] docs: Move QTest documentation to its own document, Paolo Bonzini, 2020/10/12
- [PULL 28/38] qdev: add "check if address free" callback for buses, Paolo Bonzini, 2020/10/12
- [PULL 16/38] docs/devel/qtest: Include libqtest API reference, Paolo Bonzini, 2020/10/12
- [PULL 33/38] device-core: use atomic_set on .realized property, Paolo Bonzini, 2020/10/12
- [PULL 32/38] scsi: switch to bus->check_address, Paolo Bonzini, 2020/10/12
- [PULL 13/38] qom: fix objects with improper parent type, Paolo Bonzini, 2020/10/12
- [PULL 38/38] meson: identify more sections of meson.build, Paolo Bonzini, 2020/10/12
- [PULL 30/38] device_core: use drain_call_rcu in in qmp_device_add, Paolo Bonzini, 2020/10/12
- [PULL 37/38] scsi/scsi_bus: fix races in REPORT LUNS,
Paolo Bonzini <=
- [PULL 34/38] scsi/scsi-bus: scsi_device_find: don't return unrealized devices, Paolo Bonzini, 2020/10/12
- [PULL 31/38] device-core: use RCU for list of children of a bus, Paolo Bonzini, 2020/10/12
- [PULL 36/38] virtio-scsi: use scsi_device_get, Paolo Bonzini, 2020/10/12
- [PULL 27/38] qemu-iotests, qtest: rewrite test 067 as a qtest, Paolo Bonzini, 2020/10/12
- [PULL 35/38] scsi/scsi_bus: Add scsi_device_get, Paolo Bonzini, 2020/10/12
- Re: [PULL v2 00/38] SCSI, qdev, qtest, meson patches for 2020-10-10, no-reply, 2020/10/12
- Re: [PULL v2 00/38] SCSI, qdev, qtest, meson patches for 2020-10-10, Peter Maydell, 2020/10/13