qemu_aio_get() does not zero allocated memory. Explicitly initialize
laiocb->co to prevent an uninitialized memory access in
qemu_laio_process_completion().
Note that this bug has never manifested itself. I guess we're lucky!
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
I challenge you to find a place where laiocb->co is initialized and then
we can drop this patch. I've double-checked and cannot find it...
block/linux-aio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/linux-aio.c b/block/linux-aio.c
index d4b61fb251..a097653be6 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -440,6 +440,7 @@ BlockAIOCB *laio_submit(BlockDriverState *bs, LinuxAioState *s, int fd,
int ret;
laiocb = qemu_aio_get(&laio_aiocb_info, bs, cb, opaque);
+ laiocb->co = NULL;
laiocb->nbytes = nb_sectors * BDRV_SECTOR_SIZE;
laiocb->ctx = s;
laiocb->ret = -EINPROGRESS;
--
2.21.0
Am 27.05.2019 um 11:23 hat Stefan Hajnoczi geschrieben: > qemu_aio_get() does not zero allocated memory. Explicitly initialize > laiocb->co to prevent an uninitialized memory access in > qemu_laio_process_completion(). > > Note that this bug has never manifested itself. I guess we're lucky! > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> That the bug never manifested itself might be because it's in an unused function. How about we finally just remove the unused callback-based laio_submit() from the code? At the time when I converted linux-aio to coroutines, someone (maybe Paolo?) insisted that we keep the old interface because we might add a new user sometime with possible shortcuts that bypass the whole coroutine path, but it hasn't happened and I think we've moved even further in the opposite direction since then. Kevin > I challenge you to find a place where laiocb->co is initialized and then > we can drop this patch. I've double-checked and cannot find it... > > block/linux-aio.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/linux-aio.c b/block/linux-aio.c > index d4b61fb251..a097653be6 100644 > --- a/block/linux-aio.c > +++ b/block/linux-aio.c > @@ -440,6 +440,7 @@ BlockAIOCB *laio_submit(BlockDriverState *bs, LinuxAioState *s, int fd, > int ret; > > laiocb = qemu_aio_get(&laio_aiocb_info, bs, cb, opaque); > + laiocb->co = NULL; > laiocb->nbytes = nb_sectors * BDRV_SECTOR_SIZE; > laiocb->ctx = s; > laiocb->ret = -EINPROGRESS; > -- > 2.21.0 >
On 30/05/19 10:42, Kevin Wolf wrote: > Am 27.05.2019 um 11:23 hat Stefan Hajnoczi geschrieben: >> qemu_aio_get() does not zero allocated memory. Explicitly initialize >> laiocb->co to prevent an uninitialized memory access in >> qemu_laio_process_completion(). >> >> Note that this bug has never manifested itself. I guess we're lucky! >> >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > That the bug never manifested itself might be because it's in an unused > function. How about we finally just remove the unused callback-based > laio_submit() from the code? > > At the time when I converted linux-aio to coroutines, someone (maybe > Paolo?) insisted that we keep the old interface because we might add a > new user sometime with possible shortcuts that bypass the whole coroutine > path, but it hasn't happened and I think we've moved even further in the > opposite direction since then. Yes, I suppose it's time. Spending time fixing bugs in dead code is always a sign that it's time. :) Paolo
On 30.05.2019 17:07, Paolo Bonzini wrote: > On 30/05/19 10:42, Kevin Wolf wrote: >> Am 27.05.2019 um 11:23 hat Stefan Hajnoczi geschrieben: >>> qemu_aio_get() does not zero allocated memory. Explicitly initialize >>> laiocb->co to prevent an uninitialized memory access in >>> qemu_laio_process_completion(). >>> >>> Note that this bug has never manifested itself. I guess we're lucky! >>> >>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> >> That the bug never manifested itself might be because it's in an unused >> function. How about we finally just remove the unused callback-based >> laio_submit() from the code? >> >> At the time when I converted linux-aio to coroutines, someone (maybe >> Paolo?) insisted that we keep the old interface because we might add a >> new user sometime with possible shortcuts that bypass the whole coroutine >> path, but it hasn't happened and I think we've moved even further in the >> opposite direction since then. > > Yes, I suppose it's time. Spending time fixing bugs in dead code is > always a sign that it's time. :) Great, I'll clean it up. Best regards, Julia Suvorova.
© 2016 - 2024 Red Hat, Inc.