[SRU,Bionic,1/1] xen-netfront: Fix hang on device removal

From: Jason Andryuk <jandryuk@gmail.com>

BugLink: http://bugs.launchpad.net/bugs/1771620

A toolstack may delete the vif frontend and backend xenstore entries
while xen-netfront is in the removal code path.  In that case, the
checks for xenbus_read_driver_state would return XenbusStateUnknown, and
xennet_remove would hang indefinitely.  This hang prevents system
shutdown.

xennet_remove must be able to handle XenbusStateUnknown, and
netback_changed must also wake up the wake_queue for that state as well.

Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Cc: Eduardo Otubo <otubo@redhat.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61)
Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
---
 drivers/net/xen-netfront.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

On 18/05/18 15:59, Joseph Salisbury wrote:
> From: Jason Andryuk <jandryuk@gmail.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1771620
> 
> A toolstack may delete the vif frontend and backend xenstore entries
> while xen-netfront is in the removal code path.  In that case, the
> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
> xennet_remove would hang indefinitely.  This hang prevents system
> shutdown.
> 
> xennet_remove must be able to handle XenbusStateUnknown, and
> netback_changed must also wake up the wake_queue for that state as well.
> 
> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Cc: Eduardo Otubo <otubo@redhat.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> (cherry picked from commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  drivers/net/xen-netfront.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 9bd7dde..5a37ee2 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -2007,7 +2007,10 @@ static void netback_changed(struct xenbus_device *dev,
>  	case XenbusStateInitialised:
>  	case XenbusStateReconfiguring:
>  	case XenbusStateReconfigured:
> +		break;
> +
>  	case XenbusStateUnknown:
> +		wake_up_all(&module_unload_q);
>  		break;
>  
>  	case XenbusStateInitWait:
> @@ -2138,7 +2141,9 @@ static int xennet_remove(struct xenbus_device *dev)
>  		xenbus_switch_state(dev, XenbusStateClosing);
>  		wait_event(module_unload_q,
>  			   xenbus_read_driver_state(dev->otherend) ==
> -			   XenbusStateClosing);
> +			   XenbusStateClosing ||
> +			   xenbus_read_driver_state(dev->otherend) ==
> +			   XenbusStateUnknown);
>  
>  		xenbus_switch_state(dev, XenbusStateClosed);
>  		wait_event(module_unload_q,
> 

Clean upstream cherry pick that fixes the issue, positive test results,
and limited to specific arch so limited regression potential.

Acked-by: Colin Ian King <colin.king@canonical.com>

On 05/18/18 16:59, Joseph Salisbury wrote:
> From: Jason Andryuk <jandryuk@gmail.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1771620
> 
> A toolstack may delete the vif frontend and backend xenstore entries
> while xen-netfront is in the removal code path.  In that case, the
> checks for xenbus_read_driver_state would return XenbusStateUnknown, and
> xennet_remove would hang indefinitely.  This hang prevents system
> shutdown.
> 
> xennet_remove must be able to handle XenbusStateUnknown, and
> netback_changed must also wake up the wake_queue for that state as well.
> 
> Fixes: 5b5971df3bc2 ("xen-netfront: remove warning when unloading module")
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Cc: Eduardo Otubo <otubo@redhat.com>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> (cherry picked from commit c2d2e6738a209f0f9dffa2dc8e7292fc45360d61)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  drivers/net/xen-netfront.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 9bd7dde..5a37ee2 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -2007,7 +2007,10 @@ static void netback_changed(struct xenbus_device *dev,
>  	case XenbusStateInitialised:
>  	case XenbusStateReconfiguring:
>  	case XenbusStateReconfigured:
> +		break;
> +
>  	case XenbusStateUnknown:
> +		wake_up_all(&module_unload_q);
>  		break;
>  
>  	case XenbusStateInitWait:
> @@ -2138,7 +2141,9 @@ static int xennet_remove(struct xenbus_device *dev)
>  		xenbus_switch_state(dev, XenbusStateClosing);
>  		wait_event(module_unload_q,
>  			   xenbus_read_driver_state(dev->otherend) ==
> -			   XenbusStateClosing);
> +			   XenbusStateClosing ||
> +			   xenbus_read_driver_state(dev->otherend) ==
> +			   XenbusStateUnknown);
>  
>  		xenbus_switch_state(dev, XenbusStateClosed);
>  		wait_event(module_unload_q,
> 

Clean cherry-pick, limited scope and tested by the reporter.

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

Applied to bionic master-next.

-Stefan