yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-pr-helper-avoid-error-on-PR-IN-command-with-zero-req.patch

ae23c9
From aec7894ed126514877d145ad4c3615a0b29e3443 Mon Sep 17 00:00:00 2001
ae23c9
From: Paolo Bonzini <pbonzini@redhat.com>
ae23c9
Date: Fri, 6 Jul 2018 17:56:57 +0200
ae23c9
Subject: [PATCH 197/268] pr-helper: avoid error on PR IN command with zero
ae23c9
 request size
ae23c9
ae23c9
RH-Author: Paolo Bonzini <pbonzini@redhat.com>
ae23c9
Message-id: <20180706175659.30615-8-pbonzini@redhat.com>
ae23c9
Patchwork-id: 81250
ae23c9
O-Subject: [RHEL7.6 qemu-kvm-rhev PATCH 7/9] pr-helper: avoid error on PR IN command with zero request size
ae23c9
Bugzilla: 1533158
ae23c9
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
ae23c9
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
ae23c9
RH-Acked-by: Michal Privoznik <mprivozn@redhat.com>
ae23c9
ae23c9
After reading a PR IN command with zero request size in prh_read_request,
ae23c9
the resp->result field will be uninitialized and the resp.sz field will
ae23c9
be also uninitialized when returning to prh_co_entry.
ae23c9
ae23c9
If resp->result == GOOD (from a previous successful reply or just luck),
ae23c9
then the assert in prh_write_response might not be triggered and
ae23c9
uninitialized response will be sent.
ae23c9
ae23c9
The fix is to remove the whole handling of sz == 0 in prh_co_entry.
ae23c9
Those errors apply only to PR OUT commands and it's perfectly okay to
ae23c9
catch them later in do_pr_out and multipath_pr_out; the check for
ae23c9
too-short parameters in fact doesn't apply in the easy SG_IO case, as
ae23c9
it can be left to the target firmware even.
ae23c9
ae23c9
The result is that prh_read_request does not fail requests anymore and
ae23c9
prh_co_entry becomes simpler.
ae23c9
ae23c9
Reported-by: Dima Stepanov <dimastep@yandex-team.ru>
ae23c9
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
ae23c9
(cherry picked from commit ee8c13b81474e002db083e9692b11c0e106a9c7f)
ae23c9
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
ae23c9
---
ae23c9
 scsi/qemu-pr-helper.c | 63 ++++++++++++++++++++++++---------------------------
ae23c9
 1 file changed, 30 insertions(+), 33 deletions(-)
ae23c9
ae23c9
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
ae23c9
index 0218d65..c89a446 100644
ae23c9
--- a/scsi/qemu-pr-helper.c
ae23c9
+++ b/scsi/qemu-pr-helper.c
ae23c9
@@ -455,6 +455,14 @@ static int multipath_pr_out(int fd, const uint8_t *cdb, uint8_t *sense,
ae23c9
     char transportids[PR_HELPER_DATA_SIZE];
ae23c9
     int r;
ae23c9
 
ae23c9
+    if (sz < PR_OUT_FIXED_PARAM_SIZE) {
ae23c9
+        /* Illegal request, Parameter list length error.  This isn't fatal;
ae23c9
+         * we have read the data, send an error without closing the socket.
ae23c9
+         */
ae23c9
+        scsi_build_sense(sense, SENSE_CODE(INVALID_PARAM_LEN));
ae23c9
+        return CHECK_CONDITION;
ae23c9
+    }
ae23c9
+
ae23c9
     switch (rq_servact) {
ae23c9
     case MPATH_PROUT_REG_SA:
ae23c9
     case MPATH_PROUT_RES_SA:
ae23c9
@@ -574,6 +582,12 @@ static int do_pr_out(int fd, const uint8_t *cdb, uint8_t *sense,
ae23c9
                      const uint8_t *param, int sz)
ae23c9
 {
ae23c9
     int resp_sz;
ae23c9
+
ae23c9
+    if ((fcntl(fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
ae23c9
+        scsi_build_sense(sense, SENSE_CODE(INVALID_OPCODE));
ae23c9
+        return CHECK_CONDITION;
ae23c9
+    }
ae23c9
+
ae23c9
 #ifdef CONFIG_MPATH
ae23c9
     if (is_mpath(fd)) {
ae23c9
         return multipath_pr_out(fd, cdb, sense, param, sz);
ae23c9
@@ -690,21 +704,6 @@ static int coroutine_fn prh_read_request(PRHelperClient *client,
ae23c9
                                  errp) < 0) {
ae23c9
             goto out_close;
ae23c9
         }
ae23c9
-        if ((fcntl(client->fd, F_GETFL) & O_ACCMODE) == O_RDONLY) {
ae23c9
-            scsi_build_sense(resp->sense, SENSE_CODE(INVALID_OPCODE));
ae23c9
-            sz = 0;
ae23c9
-        } else if (sz < PR_OUT_FIXED_PARAM_SIZE) {
ae23c9
-            /* Illegal request, Parameter list length error.  This isn't fatal;
ae23c9
-             * we have read the data, send an error without closing the socket.
ae23c9
-             */
ae23c9
-            scsi_build_sense(resp->sense, SENSE_CODE(INVALID_PARAM_LEN));
ae23c9
-            sz = 0;
ae23c9
-        }
ae23c9
-        if (sz == 0) {
ae23c9
-            resp->result = CHECK_CONDITION;
ae23c9
-            close(client->fd);
ae23c9
-            client->fd = -1;
ae23c9
-        }
ae23c9
     }
ae23c9
 
ae23c9
     req->fd = client->fd;
ae23c9
@@ -785,25 +784,23 @@ static void coroutine_fn prh_co_entry(void *opaque)
ae23c9
             break;
ae23c9
         }
ae23c9
 
ae23c9
-        if (sz > 0) {
ae23c9
-            num_active_sockets++;
ae23c9
-            if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
ae23c9
-                r = do_pr_out(req.fd, req.cdb, resp.sense,
ae23c9
-                              client->data, sz);
ae23c9
-                resp.sz = 0;
ae23c9
-            } else {
ae23c9
-                resp.sz = sizeof(client->data);
ae23c9
-                r = do_pr_in(req.fd, req.cdb, resp.sense,
ae23c9
-                             client->data, &resp.sz);
ae23c9
-                resp.sz = MIN(resp.sz, sz);
ae23c9
-            }
ae23c9
-            num_active_sockets--;
ae23c9
-            close(req.fd);
ae23c9
-            if (r == -1) {
ae23c9
-                break;
ae23c9
-            }
ae23c9
-            resp.result = r;
ae23c9
+        num_active_sockets++;
ae23c9
+        if (req.cdb[0] == PERSISTENT_RESERVE_OUT) {
ae23c9
+            r = do_pr_out(req.fd, req.cdb, resp.sense,
ae23c9
+                          client->data, sz);
ae23c9
+            resp.sz = 0;
ae23c9
+        } else {
ae23c9
+            resp.sz = sizeof(client->data);
ae23c9
+            r = do_pr_in(req.fd, req.cdb, resp.sense,
ae23c9
+                         client->data, &resp.sz);
ae23c9
+            resp.sz = MIN(resp.sz, sz);
ae23c9
+        }
ae23c9
+        num_active_sockets--;
ae23c9
+        close(req.fd);
ae23c9
+        if (r == -1) {
ae23c9
+            break;
ae23c9
         }
ae23c9
+        resp.result = r;
ae23c9
 
ae23c9
         if (prh_write_response(client, &req, &resp, &local_err) < 0) {
ae23c9
             break;
ae23c9
-- 
ae23c9
1.8.3.1
ae23c9