From patchwork Fri Jul 19 17:20:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 11050469 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9985113A4 for ; Fri, 19 Jul 2019 17:20:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7C5202873E for ; Fri, 19 Jul 2019 17:20:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6D233288C5; Fri, 19 Jul 2019 17:20:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0D7702873E for ; Fri, 19 Jul 2019 17:20:27 +0000 (UTC) Received: from localhost ([::1]:47248 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoWYV-0001iy-85 for patchwork-qemu-devel@patchwork.kernel.org; Fri, 19 Jul 2019 13:20:27 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53128) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hoWYN-0001BA-OO for qemu-devel@nongnu.org; Fri, 19 Jul 2019 13:20:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hoWYM-0003w5-Lc for qemu-devel@nongnu.org; Fri, 19 Jul 2019 13:20:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42916) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hoWYJ-0003th-Ry; Fri, 19 Jul 2019 13:20:16 -0400 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7322081111; Fri, 19 Jul 2019 17:20:14 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-46.phx2.redhat.com [10.3.116.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1A1F52B1D8; Fri, 19 Jul 2019 17:20:04 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 19 Jul 2019 12:20:01 -0500 Message-Id: <20190719172001.19770-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 19 Jul 2019 17:20:14 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-4.1 v2] nbd: Initialize reply on failure X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Thomas Huth , "open list:Network Block Dev..." , Max Reitz , Andrey Shinkevich , philmd@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP We've had two separate reports of different callers running into use of uninitialized data if s->quit is set (one detected by gcc -O3, another by valgrind), due to checking 'nbd_reply_is_simple(reply) || s->quit' in the wrong order. Rather than chasing down which callers need to pre-initialize reply, and whether there are any other uninitialized uses, it's easier to guarantee that reply will always be set by nbd_co_receive_one_chunk() even on failure. The uninitialized use happens to be harmless (the only time the variable is uninitialized is if s->quit is set, so the conditional results in the same action regardless of what was read from reply), and was introduced in commit 65e01d47. In fixing the problem, it can also be seen that all (one) callers pass in a non-NULL reply, so there is a dead condtional to also be cleaned up. Reported-by: Thomas Huth Reported-by: Andrey Shinkevich Signed-off-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Andrey Shinkevich --- block/nbd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/block/nbd.c b/block/nbd.c index 81edabbf35ed..57c1a205811a 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -640,12 +640,11 @@ static coroutine_fn int nbd_co_receive_one_chunk( request_ret, qiov, payload, errp); if (ret < 0) { + memset(reply, 0, sizeof(*reply)); s->quit = true; } else { /* For assert at loop start in nbd_connection_entry */ - if (reply) { - *reply = s->reply; - } + *reply = s->reply; s->reply.handle = 0; }