Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] aio-posix: avoid NULL pointer dereference in aio_epoll_update

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

On Mon, Nov 07, 2016 at 10:33:33AM +0100, Paolo Bonzini wrote:
> aio_epoll_update dereferences parameter "node", but it could have been NULL
> if deleting an fd handler that was not registered in the first place.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  aio-posix.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/aio-posix.c b/aio-posix.c
> index 4ef34dd..ec908f7 100644
> --- a/aio-posix.c
> +++ b/aio-posix.c
> @@ -217,21 +217,24 @@ void aio_set_fd_handler(AioContext *ctx,
>  
>      /* Are we deleting the fd handler? */
>      if (!io_read && !io_write) {
> -        if (node) {
> -            g_source_remove_poll(&ctx->source, &node->pfd);
> -
> -            /* If the lock is held, just mark the node as deleted */
> -            if (ctx->walking_handlers) {
> -                node->deleted = 1;
> -                node->pfd.revents = 0;
> -            } else {
> -                /* Otherwise, delete it for real.  We can't just mark it as
> -                 * deleted because deleted nodes are only cleaned up after
> -                 * releasing the walking_handlers lock.
> -                 */
> -                QLIST_REMOVE(node, node);
> -                deleted = true;
> -            }
> +        if (node == NULL) {
> +            return;
> +        }
> +
> +        node->pfd.events = 0;

^--- is this left over from debugging...

> +        g_source_remove_poll(&ctx->source, &node->pfd);
> +
> +        /* If the lock is held, just mark the node as deleted */
> +        if (ctx->walking_handlers) {
> +            node->deleted = 1;
> +            node->pfd.revents = 0;

...the original code clears revents here?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]