Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1642237 > unrolled thread

[PATCH 0/3] xen/blkback: several fixes of resource management

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] xen/blkback: several fixes of resource management Juergen Gross <jgross@suse.com> - 2017-05-16 08:30 +0200
    [PATCH 3/3] xen/blkback: don't use xen_blkif_get() in xen-blkback kthread Juergen Gross <jgross@suse.com> - 2017-05-16 08:30 +0200
    [PATCH 2/3] xen/blkback: don't free be structure too early Juergen Gross <jgross@suse.com> - 2017-05-16 08:30 +0200
    Re: [PATCH 0/3] xen/blkback: several fixes of resource management Steven Haigh <netwiz@crc.id.au> - 2017-05-18 04:10 +0200
    Re: [PATCH 0/3] xen/blkback: several fixes of resource management Roger Pau Monné <roger.pau@citrix.com> - 2017-05-18 17:00 +0200
      Re: [PATCH 0/3] xen/blkback: several fixes of resource management Juergen Gross <jgross@suse.com> - 2017-05-18 17:10 +0200

#1642237 — [PATCH 0/3] xen/blkback: several fixes of resource management

Destroying a Xen guest domain while it was doing I/Os via xen-blkback
leaked several resources, including references of the guest's memory
pages.

This patch series addresses those leaks by correcting usage of
reference counts and the sequence when to free which resource.

The series applies on top of commit 2d4456c73a487abe ("block:
xen-blkback: add null check to avoid null pointer dereference") in
Jens Axboe's tree kernel/git/axboe/linux-block.git

Juergen Gross (3):
  xen/blkback: fix disconnect while I/Os in flight
  xen/blkback: don't free be structure too early
  xen/blkback: don't use xen_blkif_get() in xen-blkback kthread

 drivers/block/xen-blkback/blkback.c |  3 ---
 drivers/block/xen-blkback/common.h  |  1 +
 drivers/block/xen-blkback/xenbus.c  | 15 ++++++++-------
 3 files changed, 9 insertions(+), 10 deletions(-)

-- 
2.12.0

[toc] | [next] | [standalone]

#1642239 — [PATCH 3/3] xen/blkback: don't use xen_blkif_get() in xen-blkback kthread

There is no need to use xen_blkif_get()/xen_blkif_put() in the kthread
of xen-blkback. Thread stopping is synchronous and using the blkif
reference counting in the kthread will avoid to ever let the reference
count drop to zero at the end of an I/O running concurrent to
disconnecting and multiple rings.

Setting ring->xenblkd to NULL after stopping the kthread isn't needed
as the kthread does this already.

Cc: stable@vger.kernel.org
Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/block/xen-blkback/blkback.c | 3 ---
 drivers/block/xen-blkback/xenbus.c  | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 726c32e35db9..6b14c509f3c7 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -609,8 +609,6 @@ int xen_blkif_schedule(void *arg)
 	unsigned long timeout;
 	int ret;
 
-	xen_blkif_get(blkif);
-
 	set_freezable();
 	while (!kthread_should_stop()) {
 		if (try_to_freeze())
@@ -665,7 +663,6 @@ int xen_blkif_schedule(void *arg)
 		print_stats(ring);
 
 	ring->xenblkd = NULL;
-	xen_blkif_put(blkif);
 
 	return 0;
 }
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 4d2f57fa35da..1dc0ff5ed912 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -255,7 +255,6 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 		if (ring->xenblkd) {
 			kthread_stop(ring->xenblkd);
 			wake_up(&ring->shutdown_wq);
-			ring->xenblkd = NULL;
 		}
 
 		/* The above kthread_stop() guarantees that at this point we
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1642240 — [PATCH 2/3] xen/blkback: don't free be structure too early

The be structure must nor be freed when freeing the blkif structure
isn't done. Otherwise a use-after-free of be when unmapping the ring
used for communicating with the frontend will occur in case of a
late call of xenblk_disconnect() (e.g. due to an I/O still active
when trying to disconnect).

Cc: stable@vger.kernel.org
Reported-by: Glenn Enright <glenn@rimuhosting.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
 drivers/block/xen-blkback/xenbus.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index e68df9de8858..4d2f57fa35da 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -315,9 +315,10 @@ static int xen_blkif_disconnect(struct xen_blkif *blkif)
 
 static void xen_blkif_free(struct xen_blkif *blkif)
 {
-
-	xen_blkif_disconnect(blkif);
+	WARN_ON(xen_blkif_disconnect(blkif));
 	xen_vbd_free(&blkif->vbd);
+	kfree(blkif->be->mode);
+	kfree(blkif->be);
 
 	/* Make sure everything is drained before shutting down */
 	kmem_cache_free(xen_blkif_cachep, blkif);
@@ -514,8 +515,6 @@ static int xen_blkbk_remove(struct xenbus_device *dev)
 		xen_blkif_put(be->blkif);
 	}
 
-	kfree(be->mode);
-	kfree(be);
 	return 0;
 }
 
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1643738

On 2017-05-16 16:23, Juergen Gross wrote:
> Destroying a Xen guest domain while it was doing I/Os via xen-blkback
> leaked several resources, including references of the guest's memory
> pages.
> 
> This patch series addresses those leaks by correcting usage of
> reference counts and the sequence when to free which resource.
> 
> The series applies on top of commit 2d4456c73a487abe ("block:
> xen-blkback: add null check to avoid null pointer dereference") in
> Jens Axboe's tree kernel/git/axboe/linux-block.git
> 
> Juergen Gross (3):
>   xen/blkback: fix disconnect while I/Os in flight
>   xen/blkback: don't free be structure too early
>   xen/blkback: don't use xen_blkif_get() in xen-blkback kthread
> 
>  drivers/block/xen-blkback/blkback.c |  3 ---
>  drivers/block/xen-blkback/common.h  |  1 +
>  drivers/block/xen-blkback/xenbus.c  | 15 ++++++++-------
>  3 files changed, 9 insertions(+), 10 deletions(-)

Tested-by: Steven Haigh <netwiz@crc.id.au>

I've had a report that a new message is logged on destroy sometimes:
vif vif-1-0 vif1.0: Guest Rx stalled

This may be a different issue - however the main fix of this patch set 
is fully functional.

-- 
Steven Haigh

Email: netwiz@crc.id.au
Web: https://www.crc.id.au
Phone: (03) 9001 6090 - 0412 935 897

[toc] | [prev] | [next] | [standalone]

#1644654

On Tue, May 16, 2017 at 08:23:17AM +0200, Juergen Gross wrote:
> Destroying a Xen guest domain while it was doing I/Os via xen-blkback
> leaked several resources, including references of the guest's memory
> pages.
> 
> This patch series addresses those leaks by correcting usage of
> reference counts and the sequence when to free which resource.
> 
> The series applies on top of commit 2d4456c73a487abe ("block:
> xen-blkback: add null check to avoid null pointer dereference") in
> Jens Axboe's tree kernel/git/axboe/linux-block.git
> 
> Juergen Gross (3):
>   xen/blkback: fix disconnect while I/Os in flight
>   xen/blkback: don't free be structure too early
>   xen/blkback: don't use xen_blkif_get() in xen-blkback kthread

All 3:

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

The comment reported by Dietmar in patch #1 would be nice to fix.

Roger.

[toc] | [prev] | [next] | [standalone]

#1644662

On 18/05/17 16:38, Roger Pau Monné wrote:
> On Tue, May 16, 2017 at 08:23:17AM +0200, Juergen Gross wrote:
>> Destroying a Xen guest domain while it was doing I/Os via xen-blkback
>> leaked several resources, including references of the guest's memory
>> pages.
>>
>> This patch series addresses those leaks by correcting usage of
>> reference counts and the sequence when to free which resource.
>>
>> The series applies on top of commit 2d4456c73a487abe ("block:
>> xen-blkback: add null check to avoid null pointer dereference") in
>> Jens Axboe's tree kernel/git/axboe/linux-block.git
>>
>> Juergen Gross (3):
>>   xen/blkback: fix disconnect while I/Os in flight
>>   xen/blkback: don't free be structure too early
>>   xen/blkback: don't use xen_blkif_get() in xen-blkback kthread
> 
> All 3:
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> The comment reported by Dietmar in patch #1 would be nice to fix.

Okay, I'll send V2 soon.


Juergen

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web