[PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

If a block device is hot-added when we are out of grants,
gnttab_grant_foreign_access fails with -ENOSPC (log message "28
granting access to ring page") in this code path:

  talk_to_blkback ->
	setup_blkring ->
		xenbus_grant_ring ->
			gnttab_grant_foreign_access

and the failing path in talk_to_blkback sets the driver_data to NULL:

 destroy_blkring:
        blkif_free(info, 0);

        mutex_lock(&blkfront_mutex);
        free_info(info);
        mutex_unlock(&blkfront_mutex);

        dev_set_drvdata(&dev->dev, NULL);

This results in a NULL pointer BUG when blkfront_remove and blkif_free
try to access the failing device's NULL struct blkfront_info.

Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>
---
 drivers/block/xen-blkfront.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 429d20131c7e..8871bf4c8a2e 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2493,6 +2493,9 @@ static int blkfront_remove(struct xenbus_device *xbdev)
 
 	dev_dbg(&xbdev->dev, "%s removed", xbdev->nodename);
 
+	if (!info)
+		return 0;
+
 	blkif_free(info, 0);
 
 	mutex_lock(&info->mutex);
-- 
2.19.0

Re: [PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Roger Pau Monné]

On Mon, Oct 15, 2018 at 03:25:08PM +0200, Vasilis Liaskovitis wrote:
> If a block device is hot-added when we are out of grants,
> gnttab_grant_foreign_access fails with -ENOSPC (log message "28
> granting access to ring page") in this code path:
> 
>   talk_to_blkback ->
> 	setup_blkring ->
> 		xenbus_grant_ring ->
> 			gnttab_grant_foreign_access
> 
> and the failing path in talk_to_blkback sets the driver_data to NULL:
> 
>  destroy_blkring:
>         blkif_free(info, 0);
> 
>         mutex_lock(&blkfront_mutex);
>         free_info(info);
>         mutex_unlock(&blkfront_mutex);
> 
>         dev_set_drvdata(&dev->dev, NULL);
> 
> This results in a NULL pointer BUG when blkfront_remove and blkif_free
> try to access the failing device's NULL struct blkfront_info.
> 
> Signed-off-by: Vasilis Liaskovitis <vliaskovitis@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

I guess this is a candidate for backporting?

Re: [PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Vasilis Liaskovitis]

On Mon, 2018-10-15 at 16:02 +0200, Roger Pau Monné wrote:
> 
> > This results in a NULL pointer BUG when blkfront_remove and
> > blkif_free
> > try to access the failing device's NULL struct blkfront_info.
> > 
> > 
> I guess this is a candidate for backporting?
> 

yes, I think so. At least for kernels >=4.5, which could face this
issue due to commit c31ecf6c12 (this frees the struct blkinfo in the
failing path of talk_to_blkback).

thanks, 

- Vasilis

Re: [PATCH v2] xen/blkfront: avoid NULL blkfront_info dereference on device removal

[Konrad Rzeszutek Wilk]

On Mon, Oct 15, 2018 at 04:25:56PM +0200, Vasilis Liaskovitis wrote:
> On Mon, 2018-10-15 at 16:02 +0200, Roger Pau Monné wrote:
> > 
> > > This results in a NULL pointer BUG when blkfront_remove and
> > > blkif_free
> > > try to access the failing device's NULL struct blkfront_info.
> > > 
> > > 
> > I guess this is a candidate for backporting?
> > 
> 
> yes, I think so. At least for kernels >=4.5, which could face this
> issue due to commit c31ecf6c12 (this frees the struct blkinfo in the
> failing path of talk_to_blkback).

OK, applied with Cc: stable@vger.kernel.org
> 
> thanks, 
> 
> - Vasilis
> 
>