From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46267) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c483K-00022B-Cw for qemu-devel@nongnu.org; Tue, 08 Nov 2016 10:11:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c483J-0003Cl-Fo for qemu-devel@nongnu.org; Tue, 08 Nov 2016 10:11:10 -0500 Date: Tue, 8 Nov 2016 23:11:04 +0800 From: Fam Zheng Message-ID: <20161108151104.GD18726@lemon> References: <20161108135524.25927-1-pbonzini@redhat.com> <20161108135524.25927-2-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161108135524.25927-2-pbonzini@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/2] aio-posix: avoid NULL pointer dereference in aio_epoll_update List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, kwolf@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 > --- > 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