Re: [Qemu-devel] [PULL 3/3] s390x/css: catch ccw sequence errors

["Andreas Färber" <afaerber@suse.de>]

Christian, Conny,

Am 23.09.2014 um 14:14 schrieb Christian Borntraeger:
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> We must not allow chains of more than 255 ccws without data transfer.
> 
> Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/css.c | 10 ++++++++++
>  hw/s390x/css.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 34637cb..b67c039 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
> @@ -294,6 +294,13 @@ static int css_interpret_ccw(SubchDev *sch, hwaddr ccw_addr)
>  
>      check_len = !((ccw.flags & CCW_FLAG_SLI) && !(ccw.flags & CCW_FLAG_DC));
>  
> +    if (!ccw.cda) {
> +        if (sch->ccw_no_data_cnt == 255) {
> +            return -EINVAL;
> +        }
> +        sch->ccw_no_data_cnt++;
> +    }
> +
>      /* Look at the command. */
>      switch (ccw.cmd_code) {
>      case CCW_CMD_NOOP:
> @@ -396,6 +403,7 @@ static void sch_handle_start_func(SubchDev *sch, ORB *orb)
>              return;
>          }
>          sch->ccw_fmt_1 = !!(orb->ctrl0 & ORB_CTRL0_MASK_FMT);
> +        sch->ccw_no_data_cnt = 0;
>      } else {
>          s->ctrl &= ~(SCSW_ACTL_SUSP | SCSW_ACTL_RESUME_PEND);
>      }
> @@ -1358,6 +1366,7 @@ void subch_device_save(SubchDev *s, QEMUFile *f)
>          qemu_put_be16(f, s->id.ciw[i].count);
>      }
>      qemu_put_byte(f, s->ccw_fmt_1);
> +    qemu_put_byte(f, s->ccw_no_data_cnt);
>      return;
>  }
>  
> @@ -1414,6 +1423,7 @@ int subch_device_load(SubchDev *s, QEMUFile *f)
>          s->id.ciw[i].count = qemu_get_be16(f);
>      }
>      s->ccw_fmt_1 = qemu_get_byte(f);
> +    s->ccw_no_data_cnt = qemu_get_byte(f);
>      return 0;
>  }
>  

You need to bump the version saved if you add fields (same as VMState),
and on load you need to check the version for whether to load the field
(or deny loading).

Regards,
Andreas

> diff --git a/hw/s390x/css.h b/hw/s390x/css.h
> index 384a455..33104ac 100644
> --- a/hw/s390x/css.h
> +++ b/hw/s390x/css.h
> @@ -78,6 +78,7 @@ struct SubchDev {
>      bool last_cmd_valid;
>      bool ccw_fmt_1;
>      bool thinint_active;
> +    uint8_t ccw_no_data_cnt;
>      /* transport-provided data: */
>      int (*ccw_cb) (SubchDev *, CCW1);
>      SenseId id;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg