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

[Fam Zheng <famz@redhat.com>]

On Tue, 11/08 14:55, 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>
> ---
>         Remove unnecessary assignment to node->pfd.revents.
> 
>  aio-posix.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 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,23 @@ 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;
> +        }
> +
> +        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;
>          }
>      } else {
>          if (node == NULL) {
> -- 
> 2.7.4
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>