| Message ID | 20220805145617.952881-2-bmeng.cn@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] util/main-loop: Fix maximum number of wait objects for win32 | expand |
Am 05.08.22 um 16:56 schrieb Bin Meng: > From: Bin Meng <bin.meng@windriver.com> > > WaitForMultipleObjects() can only wait for MAXIMUM_WAIT_OBJECTS > object handles. Correct the event array size in aio_poll() and > add a assert() to ensure it does not cause out of bound access. > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > --- > > util/aio-win32.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/util/aio-win32.c b/util/aio-win32.c > index 44003d645e..8cf5779567 100644 > --- a/util/aio-win32.c > +++ b/util/aio-win32.c > @@ -326,7 +326,7 @@ void aio_dispatch(AioContext *ctx) > bool aio_poll(AioContext *ctx, bool blocking) > { > AioHandler *node; > - HANDLE events[MAXIMUM_WAIT_OBJECTS + 1]; > + HANDLE events[MAXIMUM_WAIT_OBJECTS]; > bool progress, have_select_revents, first; > int count; > int timeout; > @@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking) > QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) { > if (!node->deleted && node->io_notify > && aio_node_check(ctx, node->is_external)) { > + assert(count < MAXIMUM_WAIT_OBJECTS); Would using g_assert for new code be better? Currently the rest of that file (and most QEMU code) uses assert. count could also be changed from int to unsigned (which matches better to the unsigned DWORD). Reviewed-by: Stefan Weil <sw@weilnetz.de>
On Fri, Aug 5, 2022 at 11:09 PM Stefan Weil <sw@weilnetz.de> wrote: > > Am 05.08.22 um 16:56 schrieb Bin Meng: > > > From: Bin Meng <bin.meng@windriver.com> > > > > WaitForMultipleObjects() can only wait for MAXIMUM_WAIT_OBJECTS > > object handles. Correct the event array size in aio_poll() and > > add a assert() to ensure it does not cause out of bound access. > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > --- > > > > util/aio-win32.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/util/aio-win32.c b/util/aio-win32.c > > index 44003d645e..8cf5779567 100644 > > --- a/util/aio-win32.c > > +++ b/util/aio-win32.c > > @@ -326,7 +326,7 @@ void aio_dispatch(AioContext *ctx) > > bool aio_poll(AioContext *ctx, bool blocking) > > { > > AioHandler *node; > > - HANDLE events[MAXIMUM_WAIT_OBJECTS + 1]; > > + HANDLE events[MAXIMUM_WAIT_OBJECTS]; > > bool progress, have_select_revents, first; > > int count; > > int timeout; > > @@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking) > > QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) { > > if (!node->deleted && node->io_notify > > && aio_node_check(ctx, node->is_external)) { > > + assert(count < MAXIMUM_WAIT_OBJECTS); > > > Would using g_assert for new code be better? Currently the rest of that > file (and most QEMU code) uses assert. Yeah, I noticed that but didn't do that because I feel it's better to be consistent, at least in this single file. Changing to g_assert() could be a future patch, if necessary. > > count could also be changed from int to unsigned (which matches better > to the unsigned DWORD). > changed in v2. > Reviewed-by: Stefan Weil <sw@weilnetz.de> Thanks! Regards, Bin
diff --git a/util/aio-win32.c b/util/aio-win32.c index 44003d645e..8cf5779567 100644 --- a/util/aio-win32.c +++ b/util/aio-win32.c @@ -326,7 +326,7 @@ void aio_dispatch(AioContext *ctx) bool aio_poll(AioContext *ctx, bool blocking) { AioHandler *node; - HANDLE events[MAXIMUM_WAIT_OBJECTS + 1]; + HANDLE events[MAXIMUM_WAIT_OBJECTS]; bool progress, have_select_revents, first; int count; int timeout; @@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking) QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) { if (!node->deleted && node->io_notify && aio_node_check(ctx, node->is_external)) { + assert(count < MAXIMUM_WAIT_OBJECTS); events[count++] = event_notifier_get_handle(node->e); } }