| Message ID | 62e87bf67b38dc8d5760586e7c96d400db854ebe.1562854091.git.alifm@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Some vfio-ccw fixes | expand |
On 7/11/19 10:28 AM, Farhan Ali wrote: > There is a small window where it's possible that we could be working > on an interrupt (queued in the workqueue) and setting up a channel > program (i.e allocating memory, pinning pages, translating address). > This can lead to allocating and freeing the channel program at the > same time and can cause memory corruption. > > Let's not call cp_free if we are currently processing a channel program. > The only way we know for sure that we don't have a thread setting > up a channel program is when the state is set to VFIO_CCW_STATE_CP_PENDING.> > Fixes: d5afd5d135c8 ("vfio-ccw: add handling for async channel instructions") > Signed-off-by: Farhan Ali <alifm@linux.ibm.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> I think this seems like a reasonable fix. Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- > drivers/s390/cio/vfio_ccw_drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c > index 4e3a903..0357165 100644 > --- a/drivers/s390/cio/vfio_ccw_drv.c > +++ b/drivers/s390/cio/vfio_ccw_drv.c > @@ -92,7 +92,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work) > (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); > if (scsw_is_solicited(&irb->scsw)) { > cp_update_scsw(&private->cp, &irb->scsw); > - if (is_final) > + if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) > cp_free(&private->cp); > } > mutex_lock(&private->io_mutex); >
diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c index 4e3a903..0357165 100644 --- a/drivers/s390/cio/vfio_ccw_drv.c +++ b/drivers/s390/cio/vfio_ccw_drv.c @@ -92,7 +92,7 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work) (SCSW_ACTL_DEVACT | SCSW_ACTL_SCHACT)); if (scsw_is_solicited(&irb->scsw)) { cp_update_scsw(&private->cp, &irb->scsw); - if (is_final) + if (is_final && private->state == VFIO_CCW_STATE_CP_PENDING) cp_free(&private->cp); } mutex_lock(&private->io_mutex);
There is a small window where it's possible that we could be working on an interrupt (queued in the workqueue) and setting up a channel program (i.e allocating memory, pinning pages, translating address). This can lead to allocating and freeing the channel program at the same time and can cause memory corruption. Let's not call cp_free if we are currently processing a channel program. The only way we know for sure that we don't have a thread setting up a channel program is when the state is set to VFIO_CCW_STATE_CP_PENDING. Fixes: d5afd5d135c8 ("vfio-ccw: add handling for async channel instructions") Signed-off-by: Farhan Ali <alifm@linux.ibm.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> --- drivers/s390/cio/vfio_ccw_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)