The channel_program struct is manipulated without a serialization mechanism to ensure consistent behavior. Take a broad stroke of putting the entire structure behind a mutex, and ensure everything that needs private->cp holds this mutex. There are a couple where the cio layer's subchannel->lock performs this role in this code, which isn't correct (it should only be used when touching the actual subchannel, like cio_enable_subchannel()), so adjust the locations where that spinlock is acquired/released to correctly coexist with this new mutex. Fixes: 0a19e61e6d4c ("vfio: ccw: introduce channel program interfaces") Cc: stable@vger.kernel.org Signed-off-by: Eric Farman --- drivers/s390/cio/vfio_ccw_drv.c | 10 +++++++++- drivers/s390/cio/vfio_ccw_fsm.c | 22 +++++++++++++++------- drivers/s390/cio/vfio_ccw_ops.c | 10 +++++++++- drivers/s390/cio/vfio_ccw_private.h | 3 +++ 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 1a095085bc72..1d8c2ed9da50 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -91,6 +91,8 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) is_final = !(scsw_actl(&irb->scsw) & (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); + + mutex_lock(&private->cp_mutex); if (scsw_is_solicited(&irb->scsw)) { cp_update_scsw(&private->cp, &irb->scsw); if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) { @@ -98,6 +100,8 @@ void vfio_ccw_sch_io_todo(struct work_struct *work) cp_is_finished = true; } } + mutex_unlock(&private->cp_mutex); + mutex_lock(&private->io_mutex); memcpy(private->io_region->irb_area, irb, sizeof(*irb)); mutex_unlock(&private->io_mutex); @@ -259,12 +263,16 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process) rc = 0; if (cio_update_schib(sch)) { - if (private) + if (private) { + spin_unlock_irqrestore(&sch->lock, flags); vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_NOT_OPER); + goto out; + } } out_unlock: spin_unlock_irqrestore(&sch->lock, flags); +out: return rc; } diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index 4d7988ea47ef..9fbe97bd23f9 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -25,17 +25,15 @@ static int fsm_io_helper(struct vfio_ccw_private *private) unsigned long flags; int ret; - spin_lock_irqsave(&sch->lock, flags); - orb = cp_get_orb(&private->cp, sch); - if (!orb) { - ret = -EIO; - goto out; - } + if (!orb) + return -EIO; VFIO_CCW_TRACE_EVENT(5, "stIO"); VFIO_CCW_TRACE_EVENT(5, dev_name(&sch->dev)); + spin_lock_irqsave(&sch->lock, flags); + /* Issue "Start Subchannel" */ ccode = ssch(sch->schid, orb); @@ -71,7 +69,6 @@ static int fsm_io_helper(struct vfio_ccw_private *private) default: ret = ccode; } -out: spin_unlock_irqrestore(&sch->lock, flags); return ret; } @@ -171,7 +168,9 @@ static void fsm_notoper(struct vfio_ccw_private *private, private->state = VFIO_CCW_STATE_NOT_OPER; /* This is usually handled during CLOSE event */ + mutex_lock(&private->cp_mutex); cp_free(&private->cp); + mutex_unlock(&private->cp_mutex); } /* @@ -252,6 +251,8 @@ static void fsm_io_request(struct vfio_ccw_private *private, private->state = VFIO_CCW_STATE_CP_PROCESSING; memcpy(scsw, io_region->scsw_area, sizeof(*scsw)); + mutex_lock(&private->cp_mutex); + if (scsw->cmd.fctl & SCSW_FCTL_START_FUNC) { orb = (union orb *)io_region->orb_area; @@ -300,6 +301,8 @@ static void fsm_io_request(struct vfio_ccw_private *private, cp_free(&private->cp); goto err_out; } + + mutex_unlock(&private->cp_mutex); return; } else if (scsw->cmd.fctl & SCSW_FCTL_HALT_FUNC) { VFIO_CCW_MSG_EVENT(2, @@ -320,6 +323,7 @@ static void fsm_io_request(struct vfio_ccw_private *private, } err_out: + mutex_unlock(&private->cp_mutex); private->state = VFIO_CCW_STATE_IDLE; trace_vfio_ccw_fsm_io_request(scsw->cmd.fctl, schid, io_region->ret_code, errstr); @@ -410,7 +414,11 @@ static void fsm_close(struct vfio_ccw_private *private, private->state = VFIO_CCW_STATE_STANDBY; spin_unlock_irq(&sch->lock); + + mutex_lock(&private->cp_mutex); cp_free(&private->cp); + mutex_unlock(&private->cp_mutex); + return; err_unlock: diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index 032a1cdf4df7..04800cfa779b 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -38,8 +38,13 @@ static void vfio_ccw_dma_unmap(struct vfio_device *vdev, u64 iova, u64 length) container_of(vdev, struct vfio_ccw_private, vdev); /* Drivers MUST unpin pages in response to an invalidation. */ - if (!cp_iova_pinned(&private->cp, iova, length)) + mutex_lock(&private->cp_mutex); + if (!cp_iova_pinned(&private->cp, iova, length)) { + mutex_unlock(&private->cp_mutex); return; + } + + mutex_unlock(&private->cp_mutex); vfio_ccw_mdev_reset(private); } @@ -50,6 +55,7 @@ static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev) container_of(vdev, struct vfio_ccw_private, vdev); mutex_init(&private->io_mutex); + mutex_init(&private->cp_mutex); private->state = VFIO_CCW_STATE_STANDBY; INIT_LIST_HEAD(&private->crw); INIT_WORK(&private->io_work, vfio_ccw_sch_io_todo); @@ -90,6 +96,7 @@ static int vfio_ccw_mdev_init_dev(struct vfio_device *vdev) out_free_cp: kfree(private->cp.guest_cp); out_free_private: + mutex_destroy(&private->cp_mutex); mutex_destroy(&private->io_mutex); return -ENOMEM; } @@ -141,6 +148,7 @@ static void vfio_ccw_mdev_release_dev(struct vfio_device *vdev) kmem_cache_free(vfio_ccw_cmd_region, private->cmd_region); kmem_cache_free(vfio_ccw_io_region, private->io_region); kfree(private->cp.guest_cp); + mutex_destroy(&private->cp_mutex); mutex_destroy(&private->io_mutex); } diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h index 0501d4bbcdbd..ac5aaa78a74b 100644 --- a/drivers/s390/cio/vfio_ccw_private.h +++ b/drivers/s390/cio/vfio_ccw_private.h @@ -94,6 +94,7 @@ struct vfio_ccw_parent { * @schib_region: MMIO region for SCHIB information * @crw_region: MMIO region for getting channel report words * @num_regions: number of additional regions + * @cp_mutex: protect against concurrent update of CP resources * @cp: channel program for the current I/O operation * @irb: irb info received from interrupt * @scsw: scsw info @@ -115,7 +116,9 @@ struct vfio_ccw_private { struct ccw_crw_region *crw_region; int num_regions; + struct mutex cp_mutex; struct channel_program cp; + struct irb irb; union scsw scsw; struct list_head crw; -- 2.53.0