+-- On Tue, 6 Sep 2016, P J P wrote --+ | From: Prasad J Pandit <p...@fedoraproject.org> | | In PVSCSI paravirtual SCSI bus, pvscsi_convert_sglist can take a very | long time or go into an infinite loop due to two different bugs: | | 1) the request descriptor data length is defined to be 64 bit. While | building SG list from a request descriptor, it gets truncated to 32bit | in routine 'pvscsi_convert_sglist'. This could lead to an infinite loop | situation for large 'dataLen' values, when data_length is cast to uint32_t | and chunk_size becomes always zero. Fix this by removing the incorrect | cast. | | 2) pvscsi_get_next_sg_elem can be called arbitrarily many times if the | element has a zero length. Get out of the loop early when this happens, | by introducing an upper limit on the number of SG list elements. | | Reported-by: Li Qiang <liqiang...@360.cn> | Signed-off-by: Prasad J Pandit <p...@fedoraproject.org> | --- | hw/scsi/vmw_pvscsi.c | 11 ++++++----- | 1 file changed, 6 insertions(+), 5 deletions(-) | | Update as per: | -> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01172.html | | diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c | index 4245c15..babac5a 100644 | --- a/hw/scsi/vmw_pvscsi.c | +++ b/hw/scsi/vmw_pvscsi.c | @@ -40,6 +40,8 @@ | #define PVSCSI_MAX_DEVS (64) | #define PVSCSI_MSIX_NUM_VECTORS (1) | | +#define PVSCSI_MAX_SG_ELEM 2048 | + | #define PVSCSI_MAX_CMD_DATA_WORDS \ | (sizeof(PVSCSICmdDescSetupRings)/sizeof(uint32_t)) | | @@ -628,17 +630,16 @@ pvscsi_queue_pending_descriptor(PVSCSIState *s, SCSIDevice **d, | static void | pvscsi_convert_sglist(PVSCSIRequest *r) | { | - int chunk_size; | + uint32_t chunk_size, elmcnt = 0; | uint64_t data_length = r->req.dataLen; | PVSCSISGState sg = r->sg; | - while (data_length) { | - while (!sg.resid) { | + while (data_length && elmcnt < PVSCSI_MAX_SG_ELEM) { | + while (!sg.resid && elmcnt++ < PVSCSI_MAX_SG_ELEM) { | pvscsi_get_next_sg_elem(&sg); | trace_pvscsi_convert_sglist(r->req.context, r->sg.dataAddr, | r->sg.resid); | } | - assert(data_length > 0); | - chunk_size = MIN((unsigned) data_length, sg.resid); | + chunk_size = MIN(data_length, sg.resid); | if (chunk_size) { | qemu_sglist_add(&r->sgl, sg.dataAddr, chunk_size); | }
Ping...! -- Prasad J Pandit / Red Hat Product Security Team 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F