[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zer
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size |
Date: |
Fri, 6 Jul 2018 19:14:21 +0200 |
After reading a PR IN command with zero request size in prh_read_request,
the resp->result field will be uninitialized and the resp.sz field will
be also uninitialized when returning to prh_co_entry.
If resp->result == GOOD (from a previous successful reply or just luck),
then the assert in prh_write_response might not be triggered and
uninitialized response will be sent.
The fix is to remove the whole handling of sz == 0 in prh_co_entry.
Those errors apply only to PR OUT commands and it's perfectly okay to
catch them later in do_pr_out and multipath_pr_out; the check for
too-short parameters in fact doesn't apply in the easy SG_IO case, as
it can be left to the target firmware even.
The result is that prh_read_request does not fail requests anymore and
prh_co_entry becomes simpler.
Reported-by: Dima Stepanov <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
scsi/qemu-pr-helper.c | 63 ++++++++++++++++++++++++---------------------------
1 file changed, 30 insertions(+), 33 deletions(-)
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index 0218d65..c89a446 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -455,6 +455,14 @@ static int multipath_pr_out(int fd, const uint8_t *cdb,
uint8_t *sense,
char transportids[PR_HELPER_DATA_SIZE];
int r;
+ if (sz < PR_OUT_FIXED_PARAM_SIZE) {
+ /* Illegal request, Parameter list length error. This isn't fatal;
+ * we have read the data, send an error without closing the socket.
+ */
+ scsi_build_sense(sense, SENSE_CODE(INVALID_PARAM_LEN));
+ return CHECK_CONDITION;
+ }
+
switch (rq_servact) {
case MPATH_PROUT_REG_SA:
case MPATH_PROUT_RES_SA:
@@ -574,6 +582,12 @@ static int do_pr_out(int fd, const uint8_t *cdb, uint8_t
*sense,
const uint8_t *param, int sz)
{
int resp_sz;
+
+ if ((fcntl(fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
+ scsi_build_sense(sense, SENSE_CODE(INVALID_OPCODE));
+ return CHECK_CONDITION;
+ }
+
#ifdef CONFIG_MPATH
if (is_mpath(fd)) {
return multipath_pr_out(fd, cdb, sense, param, sz);
@@ -690,21 +704,6 @@ static int coroutine_fn prh_read_request(PRHelperClient
*client,
errp) < 0) {
goto out_close;
}
- if ((fcntl(client->fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
- scsi_build_sense(resp->sense, SENSE_CODE(INVALID_OPCODE));
- sz = 0;
- } else if (sz < PR_OUT_FIXED_PARAM_SIZE) {
- /* Illegal request, Parameter list length error. This isn't fatal;
- * we have read the data, send an error without closing the socket.
- */
- scsi_build_sense(resp->sense, SENSE_CODE(INVALID_PARAM_LEN));
- sz = 0;
- }
- if (sz == 0) {
- resp->result = CHECK_CONDITION;
- close(client->fd);
- client->fd = -1;
- }
}
req->fd = client->fd;
@@ -785,25 +784,23 @@ static void coroutine_fn prh_co_entry(void *opaque)
break;
}
- if (sz > 0) {
- num_active_sockets++;
- if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
- r = do_pr_out(req.fd, req.cdb, resp.sense,
- client->data, sz);
- resp.sz = 0;
- } else {
- resp.sz = sizeof(client->data);
- r = do_pr_in(req.fd, req.cdb, resp.sense,
- client->data, &resp.sz);
- resp.sz = MIN(resp.sz, sz);
- }
- num_active_sockets--;
- close(req.fd);
- if (r == -1) {
- break;
- }
- resp.result = r;
+ num_active_sockets++;
+ if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
+ r = do_pr_out(req.fd, req.cdb, resp.sense,
+ client->data, sz);
+ resp.sz = 0;
+ } else {
+ resp.sz = sizeof(client->data);
+ r = do_pr_in(req.fd, req.cdb, resp.sense,
+ client->data, &resp.sz);
+ resp.sz = MIN(resp.sz, sz);
+ }
+ num_active_sockets--;
+ close(req.fd);
+ if (r == -1) {
+ break;
}
+ resp.result = r;
if (prh_write_response(client, &req, &resp, &local_err) < 0) {
break;
--
1.8.3.1
- [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 4/8] i386: fix '-cpu ?' output for host cpu type, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 1/8] pr-helper: avoid error on PR IN command with zero request size,
Paolo Bonzini <=
- [Qemu-devel] [PULL 3/8] qtest: Use cpu address space instead of system memory, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 2/8] pr-helper: Rework socket path handling, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 5/8] qemu-char: check errno together with ret < 0, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 6/8] pr-manager-helper: fix memory leak on event, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 8/8] checkpatch: handle token pasting better, Paolo Bonzini, 2018/07/06
- [Qemu-devel] [PULL 7/8] ioapic: remove useless lower bounds check, Paolo Bonzini, 2018/07/06
- Re: [Qemu-devel] [PULL 0/8] Bug fixes for 2018-07-06, Peter Maydell, 2018/07/06