scsi: pvscsi: request descriptor data_length to 32 bit

Commit Message

Prasad Pandit Sept. 5, 2016, 12:58 p.m. UTC

+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+
| Without a public spec it's hard, but I guess 2048 is more than enough.

===

Does this look okay?
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Comments

Paolo Bonzini Sept. 5, 2016, 1:02 p.m. UTC | #1

On 05/09/2016 14:58, P J P wrote:
> +-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+
> | Without a public spec it's hard, but I guess 2048 is more than enough.
> 
> ===
> diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
> index 4245c15..4823b9d 100644
> --- a/hw/scsi/vmw_pvscsi.c
> +++ b/hw/scsi/vmw_pvscsi.c
> @@ -628,17 +628,16 @@ pvscsi_queue_pending_descriptor(PVSCSIState *s, SCSIDevice **d,
>  static void
>  pvscsi_convert_sglist(PVSCSIRequest *r)
>  {
> -    int chunk_size;
> +    int chunk_size, n = 0;

chunk_size should be uint32_t.

>      uint64_t data_length = r->req.dataLen;
>      PVSCSISGState sg = r->sg;
> -    while (data_length) {
> -        while (!sg.resid) {
> +    while (data_length && n < 2048) {
> +        while (!sg.resid && n++ < 2048) {
>              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);
>          }
> ===
> 
> Does this look okay?

Yes, just change 2048 to a #define PVSCSI_MAX_SG_ELEM.

Paolo

> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
>

Prasad Pandit Sept. 5, 2016, 8:52 p.m. UTC | #2

+-- On Mon, 5 Sep 2016, Paolo Bonzini wrote --+
| chunk_size should be uint32_t.
| 
| > -    while (data_length) {
| > -        while (!sg.resid) {
| > +    while (data_length && n < 2048) {
| > +        while (!sg.resid && n++ < 2048) {
| >              pvscsi_get_next_sg_elem(&sg);
| > -        assert(data_length > 0);
| > -        chunk_size = MIN((unsigned) data_length, sg.resid);
| > +        chunk_size = MIN(data_length, sg.resid);
| > Does this look okay?
| 
| Yes, just change 2048 to a #define PVSCSI_MAX_SG_ELEM.

  Done. I've sent a revised patch v2.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

Patch

===
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 4245c15..4823b9d 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -628,17 +628,16 @@  pvscsi_queue_pending_descriptor(PVSCSIState *s, SCSIDevice **d,
 static void
 pvscsi_convert_sglist(PVSCSIRequest *r)
 {
-    int chunk_size;
+    int chunk_size, n = 0;
     uint64_t data_length = r->req.dataLen;
     PVSCSISGState sg = r->sg;
-    while (data_length) {
-        while (!sg.resid) {
+    while (data_length && n < 2048) {
+        while (!sg.resid && n++ < 2048) {
             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);
         }