* [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings
@ 2016-09-01 11:00 P J P
2016-09-01 11:19 ` Paolo Bonzini
2016-09-01 11:58 ` Dmitry Fleytman
0 siblings, 2 replies; 3+ messages in thread
From: P J P @ 2016-09-01 11:00 UTC (permalink / raw)
To: Qemu Developers
Cc: Dmitry Fleytman, Paolo Bonzini, Tom Victor, Li Qiang,
Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
Vmware Paravirtual SCSI emulation uses command descriptors to
process SCSI commands. These descriptors come with their ring
buffers. A guest could set the page count for these rings to
an arbitrary value, leading to infinite loop or OOB access.
Add check to avoid it.
Reported-by: Tom Victor <vv474172261@gmail.com>
Reported-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
hw/scsi/vmw_pvscsi.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
Update per review
-> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00019.html
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 5116f4a..4245c15 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -152,7 +152,7 @@ pvscsi_log2(uint32_t input)
return log;
}
-static int
+static void
pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
{
int i;
@@ -160,10 +160,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
uint32_t req_ring_size, cmp_ring_size;
m->rs_pa = ri->ringsStatePPN << VMW_PAGE_SHIFT;
- if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)
- || (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)) {
- return -1;
- }
req_ring_size = ri->reqRingNumPages * PVSCSI_MAX_NUM_REQ_ENTRIES_PER_PAGE;
cmp_ring_size = ri->cmpRingNumPages * PVSCSI_MAX_NUM_CMP_ENTRIES_PER_PAGE;
txr_len_log2 = pvscsi_log2(req_ring_size - 1);
@@ -195,8 +191,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
/* Flush ring state page changes */
smp_wmb();
-
- return 0;
}
static int
@@ -746,7 +740,7 @@ pvscsi_dbg_dump_tx_rings_config(PVSCSICmdDescSetupRings *rc)
trace_pvscsi_tx_rings_num_pages("Confirm Ring", rc->cmpRingNumPages);
for (i = 0; i < rc->cmpRingNumPages; i++) {
- trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->reqRingPPNs[i]);
+ trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->cmpRingPPNs[i]);
}
}
@@ -779,10 +773,15 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_SETUP_RINGS");
+ if (!rc->reqRingNumPages
+ || rc->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES
+ || !rc->cmpRingNumPages
+ || rc->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) {
+ return PVSCSI_COMMAND_PROCESSING_FAILED;
+ }
+
pvscsi_dbg_dump_tx_rings_config(rc);
- if (pvscsi_ring_init_data(&s->rings, rc) < 0) {
- return PVSCSI_COMMAND_PROCESSING_FAILED;
- }
+ pvscsi_ring_init_data(&s->rings, rc);
s->rings_info_valid = TRUE;
return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
--
2.5.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings
2016-09-01 11:00 [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings P J P
@ 2016-09-01 11:19 ` Paolo Bonzini
2016-09-01 11:58 ` Dmitry Fleytman
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2016-09-01 11:19 UTC (permalink / raw)
To: P J P, Qemu Developers
Cc: Dmitry Fleytman, Tom Victor, Prasad J Pandit, Li Qiang,
qemu-stable
On 01/09/2016 13:00, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> Vmware Paravirtual SCSI emulation uses command descriptors to
> process SCSI commands. These descriptors come with their ring
> buffers. A guest could set the page count for these rings to
> an arbitrary value, leading to infinite loop or OOB access.
> Add check to avoid it.
>
> Reported-by: Tom Victor <vv474172261@gmail.com>
> Reported-by: Li Qiang <liqiang6-s@360.cn>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
> hw/scsi/vmw_pvscsi.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> Update per review
> -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00019.html
>
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 5116f4a..4245c15 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -152,7 +152,7 @@ pvscsi_log2(uint32_t input)
> return log;
> }
>
> -static int
> +static void
> pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
> {
> int i;
> @@ -160,10 +160,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
> uint32_t req_ring_size, cmp_ring_size;
> m->rs_pa = ri->ringsStatePPN << VMW_PAGE_SHIFT;
>
> - if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)
> - || (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)) {
> - return -1;
> - }
> req_ring_size = ri->reqRingNumPages * PVSCSI_MAX_NUM_REQ_ENTRIES_PER_PAGE;
> cmp_ring_size = ri->cmpRingNumPages * PVSCSI_MAX_NUM_CMP_ENTRIES_PER_PAGE;
> txr_len_log2 = pvscsi_log2(req_ring_size - 1);
> @@ -195,8 +191,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
>
> /* Flush ring state page changes */
> smp_wmb();
> -
> - return 0;
> }
>
> static int
> @@ -746,7 +740,7 @@ pvscsi_dbg_dump_tx_rings_config(PVSCSICmdDescSetupRings *rc)
>
> trace_pvscsi_tx_rings_num_pages("Confirm Ring", rc->cmpRingNumPages);
> for (i = 0; i < rc->cmpRingNumPages; i++) {
> - trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->reqRingPPNs[i]);
> + trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->cmpRingPPNs[i]);
> }
> }
>
> @@ -779,10 +773,15 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
>
> trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_SETUP_RINGS");
>
> + if (!rc->reqRingNumPages
> + || rc->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES
> + || !rc->cmpRingNumPages
> + || rc->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) {
> + return PVSCSI_COMMAND_PROCESSING_FAILED;
> + }
> +
> pvscsi_dbg_dump_tx_rings_config(rc);
> - if (pvscsi_ring_init_data(&s->rings, rc) < 0) {
> - return PVSCSI_COMMAND_PROCESSING_FAILED;
> - }
> + pvscsi_ring_init_data(&s->rings, rc);
>
> s->rings_info_valid = TRUE;
> return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
>
Queued for 2.8, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings
2016-09-01 11:00 [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings P J P
2016-09-01 11:19 ` Paolo Bonzini
@ 2016-09-01 11:58 ` Dmitry Fleytman
1 sibling, 0 replies; 3+ messages in thread
From: Dmitry Fleytman @ 2016-09-01 11:58 UTC (permalink / raw)
To: P J P; +Cc: Qemu Developers, Paolo Bonzini, Tom Victor, Li Qiang,
Prasad J Pandit
Reviewed-by: Dmitry Fleytman <dmitry@daynix.com>
> On 1 Sep 2016, at 14:00 PM, P J P <ppandit@redhat.com> wrote:
>
> From: Prasad J Pandit <pjp@fedoraproject.org>
>
> Vmware Paravirtual SCSI emulation uses command descriptors to
> process SCSI commands. These descriptors come with their ring
> buffers. A guest could set the page count for these rings to
> an arbitrary value, leading to infinite loop or OOB access.
> Add check to avoid it.
>
> Reported-by: Tom Victor <vv474172261@gmail.com>
> Reported-by: Li Qiang <liqiang6-s@360.cn>
> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
> hw/scsi/vmw_pvscsi.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> Update per review
> -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg00019.html
>
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 5116f4a..4245c15 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -152,7 +152,7 @@ pvscsi_log2(uint32_t input)
> return log;
> }
>
> -static int
> +static void
> pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
> {
> int i;
> @@ -160,10 +160,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
> uint32_t req_ring_size, cmp_ring_size;
> m->rs_pa = ri->ringsStatePPN << VMW_PAGE_SHIFT;
>
> - if ((ri->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)
> - || (ri->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES)) {
> - return -1;
> - }
> req_ring_size = ri->reqRingNumPages * PVSCSI_MAX_NUM_REQ_ENTRIES_PER_PAGE;
> cmp_ring_size = ri->cmpRingNumPages * PVSCSI_MAX_NUM_CMP_ENTRIES_PER_PAGE;
> txr_len_log2 = pvscsi_log2(req_ring_size - 1);
> @@ -195,8 +191,6 @@ pvscsi_ring_init_data(PVSCSIRingInfo *m, PVSCSICmdDescSetupRings *ri)
>
> /* Flush ring state page changes */
> smp_wmb();
> -
> - return 0;
> }
>
> static int
> @@ -746,7 +740,7 @@ pvscsi_dbg_dump_tx_rings_config(PVSCSICmdDescSetupRings *rc)
>
> trace_pvscsi_tx_rings_num_pages("Confirm Ring", rc->cmpRingNumPages);
> for (i = 0; i < rc->cmpRingNumPages; i++) {
> - trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->reqRingPPNs[i]);
> + trace_pvscsi_tx_rings_ppn("Confirm Ring", rc->cmpRingPPNs[i]);
> }
> }
>
> @@ -779,10 +773,15 @@ pvscsi_on_cmd_setup_rings(PVSCSIState *s)
>
> trace_pvscsi_on_cmd_arrived("PVSCSI_CMD_SETUP_RINGS");
>
> + if (!rc->reqRingNumPages
> + || rc->reqRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES
> + || !rc->cmpRingNumPages
> + || rc->cmpRingNumPages > PVSCSI_SETUP_RINGS_MAX_NUM_PAGES) {
> + return PVSCSI_COMMAND_PROCESSING_FAILED;
> + }
> +
> pvscsi_dbg_dump_tx_rings_config(rc);
> - if (pvscsi_ring_init_data(&s->rings, rc) < 0) {
> - return PVSCSI_COMMAND_PROCESSING_FAILED;
> - }
> + pvscsi_ring_init_data(&s->rings, rc);
>
> s->rings_info_valid = TRUE;
> return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-09-01 11:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-01 11:00 [Qemu-devel] [PATCH v3] scsi: check page count while initialising descriptor rings P J P
2016-09-01 11:19 ` Paolo Bonzini
2016-09-01 11:58 ` Dmitry Fleytman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).