[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-trivial] [PATCH] scsi: Guard against buflen exceeding req->cmd.xfe
From: |
Thomas Higdon |
Subject: |
[Qemu-trivial] [PATCH] scsi: Guard against buflen exceeding req->cmd.xfer in scsi_disk_emulate_command |
Date: |
Tue, 24 Jan 2012 12:19:44 -0500 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
On Tue, Jan 24, 2012 at 08:53:03AM -0500, Kevin Wolf wrote:
> Am 23.01.2012 18:15, schrieb Thomas Higdon:
> > This prevents the emulated SCSI device from trying to DMA more bytes to the
> > initiator than are expected. Without this, the SCRIPTS code in the emulated
> > LSI
> > device eventually raises a DMA interrupt for a data overrun when an INQUIRY
> > command whose buflen exceeds req->cmd.xfer is processed. It's the
> > responsibility of the client to provide a request buffer and allocation
> > length that are large enough for the result of the command.
> >
> > Signed-off-by: Thomas Higdon <address@hidden>
> > ---
> > hw/scsi-disk.c | 3 +++
> > 1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> > index 5d8bf53..71fe2a3 100644
> > --- a/hw/scsi-disk.c
> > +++ b/hw/scsi-disk.c
> > @@ -477,6 +477,9 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> > uint8_t *outbuf)
> > "buffer size %zd\n", page_code, req->cmd.xfer);
> > return -1;
> > }
> > + if (buflen > req->cmd.xfer) {
> > + buflen = req->cmd.xfer;
> > + }
> > /* done with EVPD */
> > return buflen;
> > }
>
> I wonder if it would make sense to make this check in a more central
> place like scsi_send_command(). This way we would avoid similar bugs in
> other commands.
Limit the return value (corresponding to the length of the buffer to be
DMAed back to the intiator) to the value in req->cmd.xfer, which is the
amount of data that the initiator expects. Eliminate now-duplicate code
that does this guarding in the functions for individual commands.
Without this, the SCRIPTS code in the emulated LSI device eventually
raises a DMA interrupt for a data overrun when an INQUIRY command whose
buflen exceeds req->cmd.xfer is processed. It's the responsibility of
the client to provide a request buffer and allocation length that are
large enough for the result of the command.
Signed-off-by: Thomas Higdon <address@hidden>
---
I agree that it's better to get this into a more general place. However,
I wasn't willing to pull the MIN statement up into scsi_send_command
because I don't understand the interplay between 'len' in that function
and r->iov.iov_len. I couldn't see that there was a general relationship
between these two variables that applied to both read/write commands and
other commands.
hw/scsi-disk.c | 10 +---------
1 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 5d8bf53..11cfe73 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -391,9 +391,6 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
uint8_t *outbuf)
}
l = strlen(s->serial);
- if (l > req->cmd.xfer) {
- l = req->cmd.xfer;
- }
if (l > 20) {
l = 20;
}
@@ -1002,9 +999,6 @@ static int scsi_disk_emulate_mode_sense(SCSIDiskReq *r,
uint8_t *outbuf)
outbuf[0] = ((buflen - 2) >> 8) & 0xff;
outbuf[1] = (buflen - 2) & 0xff;
}
- if (buflen > r->req.cmd.xfer) {
- buflen = r->req.cmd.xfer;
- }
return buflen;
}
@@ -1038,9 +1032,6 @@ static int scsi_disk_emulate_read_toc(SCSIRequest *req,
uint8_t *outbuf)
default:
return -1;
}
- if (toclen > req->cmd.xfer) {
- toclen = req->cmd.xfer;
- }
return toclen;
}
@@ -1251,6 +1242,7 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
scsi_check_condition(r, SENSE_CODE(INVALID_OPCODE));
return -1;
}
+ buflen = MIN(buflen, req->cmd.xfer);
return buflen;
not_ready:
--
1.7.0.4