* [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
@ 2018-06-11 12:42 Jie Wang
2018-06-11 15:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: Jie Wang @ 2018-06-11 12:42 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: famz, stefanha, eblake, eric.fangyi, wu.wubin, wangjie88
if laio_init create linux_aio failed and return NULL, NULL pointer
dereference will occur when laio_attach_aio_context dereference
linux_aio in aio_get_linux_aio. Let's avoid it and report error.
Signed-off-by: Jie Wang <wangjie88@huawei.com>
---
block/file-posix.c | 19 +++++++++++++++++--
util/async.c | 5 ++++-
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 513d371bb1..653017d7a5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1665,6 +1665,11 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
#ifdef CONFIG_LINUX_AIO
} else if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
+ if (!aio) {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ return -EIO;
+ }
assert(qiov->size == bytes);
return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
#endif
@@ -1695,7 +1700,12 @@ static void raw_aio_plug(BlockDriverState *bs)
BDRVRawState *s = bs->opaque;
if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
- laio_io_plug(bs, aio);
+ if (aio) {
+ laio_io_plug(bs, aio);
+ } else {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ }
}
#endif
}
@@ -1706,7 +1716,12 @@ static void raw_aio_unplug(BlockDriverState *bs)
BDRVRawState *s = bs->opaque;
if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
- laio_io_unplug(bs, aio);
+ if (aio) {
+ laio_io_unplug(bs, aio);
+ } else {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ }
}
#endif
}
diff --git a/util/async.c b/util/async.c
index 03f62787f2..08d71340f8 100644
--- a/util/async.c
+++ b/util/async.c
@@ -327,8 +327,11 @@ LinuxAioState *aio_get_linux_aio(AioContext *ctx)
{
if (!ctx->linux_aio) {
ctx->linux_aio = laio_init();
- laio_attach_aio_context(ctx->linux_aio, ctx);
+ if (ctx->linux_aio) {
+ laio_attach_aio_context(ctx->linux_aio, ctx);
+ }
}
+
return ctx->linux_aio;
}
#endif
--
2.15.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
2018-06-11 12:42 [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference Jie Wang
@ 2018-06-11 15:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-06-11 15:34 UTC (permalink / raw)
To: Jie Wang, qemu-devel, qemu-block; +Cc: famz, eric.fangyi, stefanha, wu.wubin
Hi Jie,
On 06/11/2018 09:42 AM, Jie Wang wrote:
> if laio_init create linux_aio failed and return NULL, NULL pointer
> dereference will occur when laio_attach_aio_context dereference
> linux_aio in aio_get_linux_aio. Let's avoid it and report error.
>
> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
> block/file-posix.c | 19 +++++++++++++++++--
> util/async.c | 5 ++++-
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 513d371bb1..653017d7a5 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1665,6 +1665,11 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
> #ifdef CONFIG_LINUX_AIO
> } else if (s->use_linux_aio) {
> LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
> + if (!aio) {
> + s->use_linux_aio = false;
> + error_report("Failed to get linux aio");
> + return -EIO;
> + }
> assert(qiov->size == bytes);
> return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
> #endif
> @@ -1695,7 +1700,12 @@ static void raw_aio_plug(BlockDriverState *bs)
> BDRVRawState *s = bs->opaque;
> if (s->use_linux_aio) {
> LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
> - laio_io_plug(bs, aio);
> + if (aio) {
> + laio_io_plug(bs, aio);
> + } else {
> + s->use_linux_aio = false;
> + error_report("Failed to get linux aio");
> + }
Can you use the same form you used in raw_co_prw() ?
if (!aio) {
s->use_linux_aio = false;
error_report("Failed to get linux aio");
return;
}
laio_io_plug(bs, aio);
This is safer in case someone are more code bellow in the future.
> }
> #endif
> }
> @@ -1706,7 +1716,12 @@ static void raw_aio_unplug(BlockDriverState *bs)
> BDRVRawState *s = bs->opaque;
> if (s->use_linux_aio) {
> LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
> - laio_io_unplug(bs, aio);
> + if (aio) {
> + laio_io_unplug(bs, aio);
> + } else {
> + s->use_linux_aio = false;
> + error_report("Failed to get linux aio");
> + }
Ditto.
> }
> #endif
> }
> diff --git a/util/async.c b/util/async.c
> index 03f62787f2..08d71340f8 100644
> --- a/util/async.c
> +++ b/util/async.c
> @@ -327,8 +327,11 @@ LinuxAioState *aio_get_linux_aio(AioContext *ctx)
> {
> if (!ctx->linux_aio) {
> ctx->linux_aio = laio_init();
> - laio_attach_aio_context(ctx->linux_aio, ctx);
> + if (ctx->linux_aio) {
> + laio_attach_aio_context(ctx->linux_aio, ctx);
> + }
> }
> +
> return ctx->linux_aio;
> }
> #endif
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
@ 2018-06-11 23:26 Jie Wang
2018-06-18 15:50 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: Jie Wang @ 2018-06-11 23:26 UTC (permalink / raw)
To: qemu-devel, qemu-block
Cc: famz, stefanha, eblake, eric.fangyi, wu.wubin, wangjie88
if laio_init create linux_aio failed and return NULL, NULL pointer
dereference will occur when laio_attach_aio_context dereference
linux_aio in aio_get_linux_aio. Let's avoid it and report error.
Signed-off-by: Jie Wang <wangjie88@huawei.com>
---
block/file-posix.c | 19 +++++++++++++++++--
util/async.c | 5 ++++-
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 513d371bb1..653017d7a5 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1665,6 +1665,11 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
#ifdef CONFIG_LINUX_AIO
} else if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
+ if (!aio) {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ return -EIO;
+ }
assert(qiov->size == bytes);
return laio_co_submit(bs, aio, s->fd, offset, qiov, type);
#endif
@@ -1695,7 +1700,12 @@ static void raw_aio_plug(BlockDriverState *bs)
BDRVRawState *s = bs->opaque;
if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
- laio_io_plug(bs, aio);
+ if (aio) {
+ laio_io_plug(bs, aio);
+ } else {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ }
}
#endif
}
@@ -1706,7 +1716,12 @@ static void raw_aio_unplug(BlockDriverState *bs)
BDRVRawState *s = bs->opaque;
if (s->use_linux_aio) {
LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
- laio_io_unplug(bs, aio);
+ if (aio) {
+ laio_io_unplug(bs, aio);
+ } else {
+ s->use_linux_aio = false;
+ error_report("Failed to get linux aio");
+ }
}
#endif
}
diff --git a/util/async.c b/util/async.c
index 03f62787f2..08d71340f8 100644
--- a/util/async.c
+++ b/util/async.c
@@ -327,8 +327,11 @@ LinuxAioState *aio_get_linux_aio(AioContext *ctx)
{
if (!ctx->linux_aio) {
ctx->linux_aio = laio_init();
- laio_attach_aio_context(ctx->linux_aio, ctx);
+ if (ctx->linux_aio) {
+ laio_attach_aio_context(ctx->linux_aio, ctx);
+ }
}
+
return ctx->linux_aio;
}
#endif
--
2.15.0.windows.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
2018-06-11 23:26 Jie Wang
@ 2018-06-18 15:50 ` Stefan Hajnoczi
2018-06-26 2:51 ` WangJie (Pluto)
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2018-06-18 15:50 UTC (permalink / raw)
To: Jie Wang; +Cc: qemu-devel, qemu-block, famz, eblake, eric.fangyi, wu.wubin
[-- Attachment #1: Type: text/plain, Size: 855 bytes --]
On Tue, Jun 12, 2018 at 07:26:25AM +0800, Jie Wang wrote:
> if laio_init create linux_aio failed and return NULL, NULL pointer
> dereference will occur when laio_attach_aio_context dereference
> linux_aio in aio_get_linux_aio. Let's avoid it and report error.
>
> Signed-off-by: Jie Wang <wangjie88@huawei.com>
> ---
> block/file-posix.c | 19 +++++++++++++++++--
> util/async.c | 5 ++++-
> 2 files changed, 21 insertions(+), 3 deletions(-)
If someone wants to split aio_get_linux_aio() into an initialization
function and a "get" function which doesn't return NULL if init
succeeded, then we can make this a bit cleaner. But it doesn't matter
at the moment since there are few callers and duplicating the NULL check
isn't too bad.
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
2018-06-18 15:50 ` Stefan Hajnoczi
@ 2018-06-26 2:51 ` WangJie (Pluto)
2018-06-27 12:11 ` Stefan Hajnoczi
0 siblings, 1 reply; 6+ messages in thread
From: WangJie (Pluto) @ 2018-06-26 2:51 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel, qemu-block, famz
Thanks Stefan, will you push it to master branch?
On 2018/6/18 23:50, Stefan Hajnoczi wrote:
> On Tue, Jun 12, 2018 at 07:26:25AM +0800, Jie Wang wrote:
>> if laio_init create linux_aio failed and return NULL, NULL pointer
>> dereference will occur when laio_attach_aio_context dereference
>> linux_aio in aio_get_linux_aio. Let's avoid it and report error.
>>
>> Signed-off-by: Jie Wang <wangjie88@huawei.com>
>> ---
>> block/file-posix.c | 19 +++++++++++++++++--
>> util/async.c | 5 ++++-
>> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> If someone wants to split aio_get_linux_aio() into an initialization
> function and a "get" function which doesn't return NULL if init
> succeeded, then we can make this a bit cleaner. But it doesn't matter
> at the moment since there are few callers and duplicating the NULL check
> isn't too bad.
>
> Thanks, applied to my block tree:
> https://github.com/stefanha/qemu/commits/block
>
> Stefan
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference
2018-06-26 2:51 ` WangJie (Pluto)
@ 2018-06-27 12:11 ` Stefan Hajnoczi
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2018-06-27 12:11 UTC (permalink / raw)
To: WangJie (Pluto); +Cc: qemu-devel, qemu-block, famz
[-- Attachment #1: Type: text/plain, Size: 643 bytes --]
On Tue, Jun 26, 2018 at 10:51:50AM +0800, WangJie (Pluto) wrote:
> Thanks Stefan, will you push it to master branch?
Hi,
I have replaced this patch with "[PATCH v4] linux-aio: properly bubble
up errors from initialization". It solves the issue at initialization
time so aio_get_linux_aio() callers don't need to check for NULL.
Kevin pointed out the other patch. Sorry that work was duplicated, it
seems that both you and the other author worked on this bug at the same
time.
Please feel free to review "[PATCH v4] linux-aio: properly bubble up
errors from initialization" and reply on that email thread if you have
any concerns.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-06-27 12:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-11 12:42 [Qemu-devel] [PATCH v2] util/async: avoid NULL pointer dereference Jie Wang
2018-06-11 15:34 ` Philippe Mathieu-Daudé
-- strict thread matches above, loose matches on Subject: below --
2018-06-11 23:26 Jie Wang
2018-06-18 15:50 ` Stefan Hajnoczi
2018-06-26 2:51 ` WangJie (Pluto)
2018-06-27 12:11 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).