Re: [PATCH v1 08/18] vfio/ccw: Check that private pointer is not NULL

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 6/2/22 1:19 PM, Eric Farman wrote:
> The subchannel->dev->drvdata pointer is set in vfio_ccw_sch_probe()
> and cleared in vfio_ccw_sch_remove(). In a purely defensive move,
> let's check that the resultant pointer exists before operating on it
> any way, since some lifecycle changes are forthcoming.
> 
> Signed-off-by: Eric Farman <farman@linux.ibm.com>
> ---
>   drivers/s390/cio/vfio_ccw_drv.c | 12 ++++++++++++
>   drivers/s390/cio/vfio_ccw_ops.c |  3 +++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
> index 3784eb4cda85..f8226db26e54 100644
> --- a/drivers/s390/cio/vfio_ccw_drv.c
> +++ b/drivers/s390/cio/vfio_ccw_drv.c
> @@ -41,6 +41,9 @@ int vfio_ccw_sch_quiesce(struct subchannel *sch)
>   	DECLARE_COMPLETION_ONSTACK(completion);
>   	int iretry, ret = 0;
>   
> +	if (!private)
> +		return 0;
> +
>   	spin_lock_irq(sch->lock);
>   	if (!sch->schib.pmcw.ena)
>   		goto out_unlock;
> @@ -132,6 +135,9 @@ static void vfio_ccw_sch_irq(struct subchannel *sch)
>   {
>   	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
>   
> +	if (WARN_ON(!private))
> +		return;
> +

I wonder, why a warning here vs quietly ignoring in all the other cases? 
(maybe add a small comment)

Also, kind of wondering what serialization is protecting the state of 
private (e.g. is it possible that you got a nonzero pointer from 
dev_get_drvdata but now it's been freed by a remove)


>   	inc_irq_stat(IRQIO_CIO);
>   	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
>   }
> @@ -260,6 +266,9 @@ static void vfio_ccw_sch_remove(struct subchannel *sch)
>   {
>   	struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
>   
> +	if (!private)
> +		return;
> +
>   	vfio_ccw_sch_quiesce(sch);
>   	mdev_unregister_device(&sch->dev);
>   
> @@ -293,6 +302,9 @@ static int vfio_ccw_sch_event(struct subchannel *sch, int process)
>   	unsigned long flags;
>   	int rc = -EAGAIN;
>   
> +	if (!private)
> +		return 0;
> +
>   	spin_lock_irqsave(sch->lock, flags);
>   	if (!device_is_registered(&sch->dev))
>   		goto out_unlock;
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index 497e1b7ffd61..9e5c184eab89 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -152,6 +152,9 @@ static void vfio_ccw_mdev_remove(struct mdev_device *mdev)
>   {
>   	struct vfio_ccw_private *private = dev_get_drvdata(mdev->dev.parent);
>   
> +	if (!private)
> +		return;
> +
>   	VFIO_CCW_MSG_EVENT(2, "sch %x.%x.%04x: remove\n",
>   			   private->sch->schid.cssid,
>   			   private->sch->schid.ssid,