| Message ID | 1411474456-12226-4-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
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;
On 09/26/2014 12:36 PM, Andreas Färber wrote: >> @@ -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). Alex brough up the same thing. As the cpu migration code is not yet upstream (in prep right now) we concluded that this is still ok. http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01094.html
Am 26.09.2014 um 12:49 schrieb Christian Borntraeger: > On 09/26/2014 12:36 PM, Andreas Färber wrote: > >>> @@ -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). > > Alex brough up the same thing. As the cpu migration code is not yet upstream (in prep right now) we concluded that this is still ok. > > http://lists.gnu.org/archive/html/qemu-devel/2014-09/msg01094.html You should justify that in each such commit message then. However, the number space is large enough to bump the number each time to avoid forgetting it later. Andreas
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; } 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;