From nobody Mon Apr 29 09:40:22 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1522365604793894.8753241892331; Thu, 29 Mar 2018 16:20:04 -0700 (PDT) Received: from localhost ([::1]:39154 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f1gpv-0005WJ-HK for importer@patchew.org; Thu, 29 Mar 2018 19:20:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f1goo-0004lo-Mn for qemu-devel@nongnu.org; Thu, 29 Mar 2018 19:18:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f1gon-0002ZP-Pr for qemu-devel@nongnu.org; Thu, 29 Mar 2018 19:18:54 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:51398 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f1goi-0002XX-Lx; Thu, 29 Mar 2018 19:18:48 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C3B2E4068031; Thu, 29 Mar 2018 23:18:44 +0000 (UTC) Received: from red.redhat.com (ovpn-121-83.rdu2.redhat.com [10.10.121.83]) by smtp.corp.redhat.com (Postfix) with ESMTP id 360FF215CDC6; Thu, 29 Mar 2018 23:18:41 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Thu, 29 Mar 2018 18:18:37 -0500 Message-Id: <20180329231837.1914680-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 29 Mar 2018 23:18:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Thu, 29 Mar 2018 23:18:44 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'eblake@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: [Qemu-devel] [PATCH for-2.12] nbd/client: Correctly handle bad server REP_META_CONTEXT X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , vsementsov@virtuozzo.com, "open list:Network Block Dev..." Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" It's never a good idea to blindly read for size bytes as returned by the server without first validating that the size is within bounds; a malicious or buggy server could cause us to hang or get out of sync from reading further messages. It may be smarter to try and teach the client to cope with unexpected context ids by silently ignoring them instead of hanging up on the server, but for now, if the server doesn't reply with exactly the one context we expect, it's easier to just give up - however, if we give up for any reason other than an I/O failure, we might as well try to politely tell the server we are quitting rather than continuing. Fix some typos in the process. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/client.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 9b9b7f0ea29..4ee1d9a4a2c 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -599,8 +599,8 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, * Set one meta context. Simple means that reply must contain zero (not * negotiated) or one (negotiated) contexts. More contexts would be consid= ered * as a protocol error. It's also implied that meta-data query equals quer= ied - * context name, so, if server replies with something different then @cont= ext, - * it considered as error too. + * context name, so, if server replies with something different than @cont= ext, + * it is considered an error too. * return 1 for successful negotiation, context_id is set * 0 if operation is unsupported, * -1 with errp set for any other error @@ -651,6 +651,14 @@ static int nbd_negotiate_simple_meta_context(QIOChanne= l *ioc, char *name; size_t len; + if (reply.length !=3D sizeof(received_id) + context_len) { + error_setg(errp, "Failed to negotiate meta context '%s', serve= r " + "answered with unexpected length %u", context, + reply.length); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) { return -1; } @@ -668,6 +676,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel= *ioc, "answered with different context '%s'", context, name); g_free(name); + nbd_send_opt_abort(ioc); return -1; } g_free(name); @@ -690,6 +699,12 @@ static int nbd_negotiate_simple_meta_context(QIOChanne= l *ioc, if (reply.type !=3D NBD_REP_ACK) { error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", reply.type, NBD_REP_ACK); + nbd_send_opt_abort(ioc); + return -1; + } + if (reply.length) { + error_setg(errp, "Unexpected length to ACK response"); + nbd_send_opt_abort(ioc); return -1; } --=20 2.14.3