Re: [RFC v1 1/1] vfio-ccw: Don't call cp_free if we are processing a channel program

[Halil Pasic <pasic@linux.ibm.com>]

On Thu, 20 Jun 2019 17:07:09 -0400
Farhan Ali <alifm@linux.ibm.com> wrote:

> There is a small window where it's possible that an interrupt can
> arrive and can call cp_free, while we are still processing a channel
> program (i.e allocating memory, pinnging pages, translating
> addresses etc). This can lead to allocating and freeing at the same
> time and can cause memory corruption.
> 
> Let's not call cp_free if we are currently processing a channel program.
> 
> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
> ---
> 
> I have been running my test overnight with this patch and I haven't
> seen the stack traces that I mentioned about earlier. I would like
> to get some reviews on this and also if this is the right thing to
> do?
> 
> Thanks
> Farhan
> 
>  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 66a66ac..61ece3f 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -88,7 +88,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_PROCESSING)

How is access to private->state correctly synchronized? And don't we
expect private->state == VFIO_CCW_STATE_CP_PENDING in case the cp was
submitted successfully with a ssch() and is done now (one way or the
other)?

Does this have something to do with 71189f2 "vfio-ccw: make it safe to
access channel programs" (Cornelia Huck, 2019-01-21)?

Regards,
Halil

>  			cp_free(&private->cp);
>  	}
>  	mutex_lock(&private->io_mutex);