bdrv_inc_in_flight and bdrv_dec_in_flight:

[Qemu-devel] [PATCH] bdrv_inc_in_flight and bdrv_dec_in_flight:

Posted by Zhengui Li 8 years, 4 months ago

Avoid empty pointer access if the bs is NULL.

Signed-off-by: Zhengui Li <lizhengui@huawei.com>
---
 block/io.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/block/io.c b/block/io.c
index ed31810..b12d7cf 100644
--- a/block/io.c
+++ b/block/io.c
@@ -492,7 +492,9 @@ static bool tracked_request_overlaps(BdrvTrackedRequest *req,
 
 void bdrv_inc_in_flight(BlockDriverState *bs)
 {
-    atomic_inc(&bs->in_flight);
+    if (bs) {
+        atomic_inc(&bs->in_flight);
+    }
 }
 
 static void dummy_bh_cb(void *opaque)
@@ -508,8 +510,10 @@ void bdrv_wakeup(BlockDriverState *bs)
 
 void bdrv_dec_in_flight(BlockDriverState *bs)
 {
-    atomic_dec(&bs->in_flight);
-    bdrv_wakeup(bs);
+    if (bs) {
+        atomic_dec(&bs->in_flight);
+        bdrv_wakeup(bs);
+    }
 }
 
 static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
-- 
1.8.3.1

Re: [Qemu-devel] [Qemu-block] [PATCH] bdrv_inc_in_flight and bdrv_dec_in_flight:

Posted by Manos Pitsidianakis 8 years, 4 months ago

On Sat, Jun 10, 2017 at 03:04:10PM +0800, Zhengui Li wrote:
>Avoid empty pointer access if the bs is NULL.

Looks like most (if not all) of the places these are called dereference 
bs anyway. Can it ever be NULL? Perhaps a check for each of those case 
(if any) would be a better idea.

>Signed-off-by: Zhengui Li <lizhengui@huawei.com>
>---
> block/io.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/block/io.c b/block/io.c
>index ed31810..b12d7cf 100644
>--- a/block/io.c
>+++ b/block/io.c
>@@ -492,7 +492,9 @@ static bool tracked_request_overlaps(BdrvTrackedRequest *req,
>
> void bdrv_inc_in_flight(BlockDriverState *bs)
> {
>-    atomic_inc(&bs->in_flight);
>+    if (bs) {
>+        atomic_inc(&bs->in_flight);
>+    }
> }
>
> static void dummy_bh_cb(void *opaque)
>@@ -508,8 +510,10 @@ void bdrv_wakeup(BlockDriverState *bs)
>
> void bdrv_dec_in_flight(BlockDriverState *bs)
> {
>-    atomic_dec(&bs->in_flight);
>-    bdrv_wakeup(bs);
>+    if (bs) {
>+        atomic_dec(&bs->in_flight);
>+        bdrv_wakeup(bs);
>+    }
> }
>
> static bool coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
>-- 
>1.8.3.1
>
>
>

Re: [Qemu-devel] [PATCH] bdrv_inc_in_flight and bdrv_dec_in_flight:

Posted by Eric Blake 8 years, 4 months ago

On 06/10/2017 02:04 AM, Zhengui Li wrote:
> Avoid empty pointer access if the bs is NULL.
> 
> Signed-off-by: Zhengui Li <lizhengui@huawei.com>
> ---
>  block/io.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

NACK; this is fixing a symptom, not the problem. If you have a coredump
due to a bad caller passing in NULL, then post the backtrace and let's
fix the broken caller instead.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org