[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH 51/78] scsi/qemu-pr-helper: Fix out-of-bounds access to trnptid_l
From: |
Michael Roth |
Subject: |
[PATCH 51/78] scsi/qemu-pr-helper: Fix out-of-bounds access to trnptid_list[] |
Date: |
Tue, 16 Jun 2020 09:15:20 -0500 |
From: Christophe de Dinechin <dinechin@redhat.com>
Compile error reported by gcc 10.0.1:
scsi/qemu-pr-helper.c: In function ‘multipath_pr_out’:
scsi/qemu-pr-helper.c:523:32: error: array subscript <unknown> is outside array
bounds of ‘struct transportid *[0]’ [-Werror=array-bounds]
523 | paramp.trnptid_list[paramp.num_transportid++] = id;
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from scsi/qemu-pr-helper.c:36:
/usr/include/mpath_persist.h:168:22: note: while referencing ‘trnptid_list’
168 | struct transportid *trnptid_list[];
| ^~~~~~~~~~~~
scsi/qemu-pr-helper.c:424:35: note: defined here ‘paramp’
424 | struct prout_param_descriptor paramp;
| ^~~~~~
This highlights an actual implementation issue in function multipath_pr_out.
The variable paramp is declared with type `struct prout_param_descriptor`,
which is a struct terminated by an empty array in mpath_persist.h:
struct transportid *trnptid_list[];
That empty array was filled with code that looked like that:
trnptid_list[paramp.descr.num_transportid++] = id;
This is an actual out-of-bounds access.
The fix is to malloc `paramp`.
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 4ce1e15fbc7266a108a7c77a3962644b3935346e)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
scsi/qemu-pr-helper.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index debb18f4aa..38c273de19 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -421,10 +421,13 @@ static int multipath_pr_out(int fd, const uint8_t *cdb,
uint8_t *sense,
int rq_servact = cdb[1];
int rq_scope = cdb[2] >> 4;
int rq_type = cdb[2] & 0xf;
- struct prout_param_descriptor paramp;
+ g_autofree struct prout_param_descriptor *paramp = NULL;
char transportids[PR_HELPER_DATA_SIZE];
int r;
+ paramp = g_malloc0(sizeof(struct prout_param_descriptor)
+ + sizeof(struct transportid *) * MPATH_MX_TIDS);
+
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.
@@ -454,10 +457,9 @@ static int multipath_pr_out(int fd, const uint8_t *cdb,
uint8_t *sense,
* used by libmpathpersist (which, of course, will immediately
* do the opposite).
*/
- memset(¶mp, 0, sizeof(paramp));
- memcpy(¶mp.key, ¶m[0], 8);
- memcpy(¶mp.sa_key, ¶m[8], 8);
- paramp.sa_flags = param[20];
+ memcpy(¶mp->key, ¶m[0], 8);
+ memcpy(¶mp->sa_key, ¶m[8], 8);
+ paramp->sa_flags = param[20];
if (sz > PR_OUT_FIXED_PARAM_SIZE) {
size_t transportid_len;
int i, j;
@@ -520,12 +522,13 @@ static int multipath_pr_out(int fd, const uint8_t *cdb,
uint8_t *sense,
return CHECK_CONDITION;
}
- paramp.trnptid_list[paramp.num_transportid++] = id;
+ assert(paramp->num_transportid < MPATH_MX_TIDS);
+ paramp->trnptid_list[paramp->num_transportid++] = id;
}
}
r = mpath_persistent_reserve_out(fd, rq_servact, rq_scope, rq_type,
- ¶mp, noisy, verbose);
+ paramp, noisy, verbose);
return mpath_reconstruct_sense(fd, r, sense);
}
#endif
--
2.17.1
- [PATCH 43/78] tcg: save vaddr temp for plugin usage, (continued)
- [PATCH 43/78] tcg: save vaddr temp for plugin usage, Michael Roth, 2020/06/16
- [PATCH 45/78] qcow2: Fix qcow2_alloc_cluster_abort() for external data file, Michael Roth, 2020/06/16
- [PATCH 41/78] s390/sclp: improve special wait psw logic, Michael Roth, 2020/06/16
- [PATCH 44/78] qcow2: update_refcount(): Reset old_table_index after qcow2_cache_put(), Michael Roth, 2020/06/16
- [PATCH 47/78] qcow2: Fix alloc_cluster_abort() for pre-existing clusters, Michael Roth, 2020/06/16
- [PATCH 46/78] iotests: Test copy offloading with external data file, Michael Roth, 2020/06/16
- [PATCH 03/78] i386: Resolve CPU models to v1 by default, Michael Roth, 2020/06/16
- [PATCH 48/78] iotests/026: Test EIO on preallocated zero cluster, Michael Roth, 2020/06/16
- [PATCH 49/78] iotests/026: Test EIO on allocation in a data-file, Michael Roth, 2020/06/16
- [PATCH 50/78] virtio: gracefully handle invalid region caches, Michael Roth, 2020/06/16
- [PATCH 51/78] scsi/qemu-pr-helper: Fix out-of-bounds access to trnptid_list[],
Michael Roth <=
- [PATCH 04/78] qapi: better document NVMe blockdev @device parameter, Michael Roth, 2020/06/16
- [PATCH 52/78] block/qcow2-threads: fix qcow2_decompress, Michael Roth, 2020/06/16
- [PATCH 53/78] job: refactor progress to separate object, Michael Roth, 2020/06/16
- [PATCH 54/78] block/block-copy: fix progress calculation, Michael Roth, 2020/06/16
- [PATCH 55/78] target/ppc: Fix rlwinm on ppc64, Michael Roth, 2020/06/16
- [PATCH 57/78] compat: disable edid on correct virtio-gpu device, Michael Roth, 2020/06/16
- [PATCH 56/78] block/io: fix bdrv_co_do_copy_on_readv, Michael Roth, 2020/06/16
- [PATCH 58/78] qga: Installer: Wait for installation to finish, Michael Roth, 2020/06/16
- [PATCH 59/78] qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error, Michael Roth, 2020/06/16
- [PATCH 05/78] target/arm: ensure we use current exception state after SCR update, Michael Roth, 2020/06/16