From nobody Fri May 3 02:23:11 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 1510931426365264.0049445510101; Fri, 17 Nov 2017 07:10:26 -0800 (PST) Received: from localhost ([::1]:46406 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiHe-0007tt-Im for importer@patchew.org; Fri, 17 Nov 2017 10:10:22 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32966) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiDd-0004Ot-6P for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFiDc-0000to-Di for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39412) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eFiDS-0000ao-NV; Fri, 17 Nov 2017 10:06:02 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D8ADA4AE9B; Fri, 17 Nov 2017 15:06:01 +0000 (UTC) Received: from red.redhat.com (ovpn-123-34.rdu2.redhat.com [10.10.123.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id D8C4F7BCE0; Fri, 17 Nov 2017 15:05:59 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 17 Nov 2017 09:05:53 -0600 Message-Id: <20171117150556.16947-2-eblake@redhat.com> In-Reply-To: <20171117150556.16947-1-eblake@redhat.com> References: <20171117150556.16947-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 17 Nov 2017 15:06:01 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 1/4] nbd: Don't crash when server reports NBD_CMD_READ failure 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: Kevin Wolf , Paolo Bonzini , "open list:Network Block Dev..." , Max Reitz 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" If a server fails a read, for example with EIO, but the connection is still live, then we would crash trying to print a non-existent error message in nbd_client_co_preadv(). For consistency, also change the error printout in nbd_read_reply_entry(), although that instance does not crash. Bug introduced in commit f140e300. Signed-off-by: Eric Blake Message-Id: <20171112013936.5942-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- block/nbd-client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/nbd-client.c b/block/nbd-client.c index bcfed0133d..9206652e45 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -78,7 +78,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaqu= e) while (!s->quit) { assert(s->reply.handle =3D=3D 0); ret =3D nbd_receive_reply(s->ioc, &s->reply, &local_err); - if (ret < 0) { + if (local_err) { error_report_err(local_err); } if (ret <=3D 0) { @@ -691,7 +691,7 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t= offset, ret =3D nbd_co_receive_cmdread_reply(client, request.handle, offset, q= iov, &local_err); - if (ret < 0) { + if (local_err) { error_report_err(local_err); } return ret; --=20 2.13.6 From nobody Fri May 3 02:23:11 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 151093143723937.77997244636015; Fri, 17 Nov 2017 07:10:37 -0800 (PST) Received: from localhost ([::1]:46408 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiHm-0007zK-E1 for importer@patchew.org; Fri, 17 Nov 2017 10:10:30 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32980) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiDe-0004Q4-CI for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFiDX-0000mm-Ob for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43032) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eFiDU-0000dw-5A; Fri, 17 Nov 2017 10:06:04 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 37D4EC06020E; Fri, 17 Nov 2017 15:06:03 +0000 (UTC) Received: from red.redhat.com (ovpn-123-34.rdu2.redhat.com [10.10.123.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 22299620BA; Fri, 17 Nov 2017 15:06:02 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 17 Nov 2017 09:05:54 -0600 Message-Id: <20171117150556.16947-3-eblake@redhat.com> In-Reply-To: <20171117150556.16947-1-eblake@redhat.com> References: <20171117150556.16947-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 17 Nov 2017 15:06:03 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 2/4] nbd/client: Use error_prepend() correctly 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 , qemu-stable@nongnu.org, "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" When using error prepend(), it is necessary to end with a space in the format string; otherwise, messages come out incorrectly, such as when connecting to a socket that hangs up immediately: can't open device nbd://localhost:10809/: Failed to read dataUnexpected end= -of-file before all bytes were read Originally botched in commit e44ed99d, then several more instances added in the meantime. Pre-existing and not fixed here: we are inconsistent on capitalization; some of our messages start with lower case, and others start with upper, although the use of error_prepend() is much nicer to read when all fragments consistently start with lower. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Message-Id: <20171113152424.25381-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Markus Armbruster --- nbd/client.c | 50 ++++++++++++++++++++++++++------------------------ 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 1880103d2a..4e15fc484d 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -79,12 +79,12 @@ static int nbd_send_option_request(QIOChannel *ioc, uin= t32_t opt, stl_be_p(&req.length, len); if (nbd_write(ioc, &req, sizeof(req), errp) < 0) { - error_prepend(errp, "Failed to send option request header"); + error_prepend(errp, "Failed to send option request header: "); return -1; } if (len && nbd_write(ioc, (char *) data, len, errp) < 0) { - error_prepend(errp, "Failed to send option request data"); + error_prepend(errp, "Failed to send option request data: "); return -1; } @@ -113,7 +113,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, ui= nt32_t opt, { QEMU_BUILD_BUG_ON(sizeof(*reply) !=3D 20); if (nbd_read(ioc, reply, sizeof(*reply), errp) < 0) { - error_prepend(errp, "failed to read option reply"); + error_prepend(errp, "failed to read option reply: "); nbd_send_opt_abort(ioc); return -1; } @@ -166,7 +166,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_op= t_reply *reply, msg =3D g_malloc(reply->length + 1); if (nbd_read(ioc, msg, reply->length, errp) < 0) { error_prepend(errp, "failed to read option error 0x%" PRIx32 - " (%s) message", + " (%s) message: ", reply->type, nbd_rep_lookup(reply->type)); goto cleanup; } @@ -277,7 +277,7 @@ static int nbd_receive_list(QIOChannel *ioc, const char= *want, bool *match, return -1; } if (nbd_read(ioc, &namelen, sizeof(namelen), errp) < 0) { - error_prepend(errp, "failed to read option name length"); + error_prepend(errp, "failed to read option name length: "); nbd_send_opt_abort(ioc); return -1; } @@ -290,7 +290,8 @@ static int nbd_receive_list(QIOChannel *ioc, const char= *want, bool *match, } if (namelen !=3D strlen(want)) { if (nbd_drop(ioc, len, errp) < 0) { - error_prepend(errp, "failed to skip export name with wrong len= gth"); + error_prepend(errp, + "failed to skip export name with wrong length: "= ); nbd_send_opt_abort(ioc); return -1; } @@ -299,14 +300,14 @@ static int nbd_receive_list(QIOChannel *ioc, const ch= ar *want, bool *match, assert(namelen < sizeof(name)); if (nbd_read(ioc, name, namelen, errp) < 0) { - error_prepend(errp, "failed to read export name"); + error_prepend(errp, "failed to read export name: "); nbd_send_opt_abort(ioc); return -1; } name[namelen] =3D '\0'; len -=3D namelen; if (nbd_drop(ioc, len, errp) < 0) { - error_prepend(errp, "failed to read export description"); + error_prepend(errp, "failed to read export description: "); nbd_send_opt_abort(ioc); return -1; } @@ -390,7 +391,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, return -1; } if (nbd_read(ioc, &type, sizeof(type), errp) < 0) { - error_prepend(errp, "failed to read info type"); + error_prepend(errp, "failed to read info type: "); nbd_send_opt_abort(ioc); return -1; } @@ -405,13 +406,13 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wa= ntname, return -1; } if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { - error_prepend(errp, "failed to read info size"); + error_prepend(errp, "failed to read info size: "); nbd_send_opt_abort(ioc); return -1; } be64_to_cpus(&info->size); if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0= ) { - error_prepend(errp, "failed to read info flags"); + error_prepend(errp, "failed to read info flags: "); nbd_send_opt_abort(ioc); return -1; } @@ -428,7 +429,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, } if (nbd_read(ioc, &info->min_block, sizeof(info->min_block), errp) < 0) { - error_prepend(errp, "failed to read info minimum block siz= e"); + error_prepend(errp, "failed to read info minimum block siz= e: "); nbd_send_opt_abort(ioc); return -1; } @@ -441,7 +442,8 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, } if (nbd_read(ioc, &info->opt_block, sizeof(info->opt_block), errp) < 0) { - error_prepend(errp, "failed to read info preferred block s= ize"); + error_prepend(errp, + "failed to read info preferred block size: "= ); nbd_send_opt_abort(ioc); return -1; } @@ -455,7 +457,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, } if (nbd_read(ioc, &info->max_block, sizeof(info->max_block), errp) < 0) { - error_prepend(errp, "failed to read info maximum block siz= e"); + error_prepend(errp, "failed to read info maximum block siz= e: "); nbd_send_opt_abort(ioc); return -1; } @@ -467,7 +469,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, default: trace_nbd_opt_go_info_unknown(type, nbd_info_lookup(type)); if (nbd_drop(ioc, len, errp) < 0) { - error_prepend(errp, "Failed to read info payload"); + error_prepend(errp, "Failed to read info payload: "); nbd_send_opt_abort(ioc); return -1; } @@ -618,7 +620,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *= name, } if (nbd_read(ioc, buf, 8, errp) < 0) { - error_prepend(errp, "Failed to read data"); + error_prepend(errp, "Failed to read data: "); goto fail; } @@ -637,7 +639,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *= name, } if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { - error_prepend(errp, "Failed to read magic"); + error_prepend(errp, "Failed to read magic: "); goto fail; } magic =3D be64_to_cpu(magic); @@ -649,7 +651,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *= name, bool fixedNewStyle =3D false; if (nbd_read(ioc, &globalflags, sizeof(globalflags), errp) < 0) { - error_prepend(errp, "Failed to read server flags"); + error_prepend(errp, "Failed to read server flags: "); goto fail; } globalflags =3D be16_to_cpu(globalflags); @@ -665,7 +667,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *= name, /* client requested flags */ clientflags =3D cpu_to_be32(clientflags); if (nbd_write(ioc, &clientflags, sizeof(clientflags), errp) < 0) { - error_prepend(errp, "Failed to send clientflags field"); + error_prepend(errp, "Failed to send clientflags field: "); goto fail; } if (tlscreds) { @@ -727,13 +729,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, /* Read the response */ if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { - error_prepend(errp, "Failed to read export length"); + error_prepend(errp, "Failed to read export length: "); goto fail; } be64_to_cpus(&info->size); if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) { - error_prepend(errp, "Failed to read export flags"); + error_prepend(errp, "Failed to read export flags: "); goto fail; } be16_to_cpus(&info->flags); @@ -750,13 +752,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, } if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { - error_prepend(errp, "Failed to read export length"); + error_prepend(errp, "Failed to read export length: "); goto fail; } be64_to_cpus(&info->size); if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) { - error_prepend(errp, "Failed to read export flags"); + error_prepend(errp, "Failed to read export flags: "); goto fail; } be32_to_cpus(&oldflags); @@ -772,7 +774,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *= name, trace_nbd_receive_negotiate_size_flags(info->size, info->flags); if (zeroes && nbd_drop(ioc, 124, errp) < 0) { - error_prepend(errp, "Failed to read reserved block"); + error_prepend(errp, "Failed to read reserved block: "); goto fail; } rc =3D 0; --=20 2.13.6 From nobody Fri May 3 02:23:11 2024 Delivered-To: importer@patchew.org Received-SPF: temperror (zoho.com: Error in retrieving data from DNS) 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=temperror (zoho.com: Error in retrieving data from DNS) 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 1510931309166910.9757850553751; Fri, 17 Nov 2017 07:08:29 -0800 (PST) Received: from localhost ([::1]:46392 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiFZ-00061V-2U for importer@patchew.org; Fri, 17 Nov 2017 10:08:13 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32937) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiDZ-0004M8-Ma for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFiDY-0000o6-Lg for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55244) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eFiDV-0000h3-OA; Fri, 17 Nov 2017 10:06:05 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E94EC7E3A8; Fri, 17 Nov 2017 15:06:04 +0000 (UTC) Received: from red.redhat.com (ovpn-123-34.rdu2.redhat.com [10.10.123.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 765A1620BA; Fri, 17 Nov 2017 15:06:03 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 17 Nov 2017 09:05:55 -0600 Message-Id: <20171117150556.16947-4-eblake@redhat.com> In-Reply-To: <20171117150556.16947-1-eblake@redhat.com> References: <20171117150556.16947-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 17 Nov 2017 15:06:04 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 3/4] nbd/client: Don't hard-disconnect on ESHUTDOWN from server 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 , qemu-stable@nongnu.org, "open list:Network Block Dev..." Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_6 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" The NBD spec says that a server may fail any transmission request with ESHUTDOWN when it is apparent that no further request from the client can be successfully honored. The client is supposed to then initiate a soft shutdown (wait for all remaining in-flight requests to be answered, then send NBD_CMD_DISC). However, since qemu's server never uses ESHUTDOWN errors, this code was mostly untested since its introduction in commit b6f5d3b5. More recently, I learned that nbdkit as the NBD server is able to send ESHUTDOWN errors, so I finally tested this code, and noticed that our client was special-casing ESHUTDOWN to cause a hard shutdown (immediate disconnect, with no NBD_CMD_DISC), but only if the server sends this error as a simple reply. Further investigation found that commit d2febedb introduced a regression where structured replies behave differently than simple replies - but that the structured reply behavior is more in line with the spec (even if we still lack code in nbd-client.c to properly quit sending further requests). So this patch reverts the portion of b6f5d3b5 that introduced an improper hard-disconnect special-case at the lower level, and leaves the future enhancement of a nicer soft-disconnect at the higher level for another day. CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake Message-Id: <20171113194857.13933-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/client.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 4e15fc484d..eea236ca06 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -996,15 +996,9 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply= , Error **errp) if (ret < 0) { break; } - trace_nbd_receive_simple_reply(reply->simple.error, nbd_err_lookup(reply->simple.error), reply->handle); - if (reply->simple.error =3D=3D NBD_ESHUTDOWN) { - /* This works even on mingw which lacks a native ESHUTDOWN */ - error_setg(errp, "server shutting down"); - return -EINVAL; - } break; case NBD_STRUCTURED_REPLY_MAGIC: ret =3D nbd_receive_structured_reply_chunk(ioc, &reply->structured= , errp); --=20 2.13.6 From nobody Fri May 3 02:23:11 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 1510931311453118.79739673018912; Fri, 17 Nov 2017 07:08:31 -0800 (PST) Received: from localhost ([::1]:46393 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiFl-0006BU-Hu for importer@patchew.org; Fri, 17 Nov 2017 10:08:25 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33038) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFiDr-0004YT-4r for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFiDn-00017i-2g for qemu-devel@nongnu.org; Fri, 17 Nov 2017 10:06:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43838) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eFiDW-0000jg-Nw; Fri, 17 Nov 2017 10:06:06 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E72DC20B19; Fri, 17 Nov 2017 15:06:05 +0000 (UTC) Received: from red.redhat.com (ovpn-123-34.rdu2.redhat.com [10.10.123.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3328E620BA; Fri, 17 Nov 2017 15:06:05 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 17 Nov 2017 09:05:56 -0600 Message-Id: <20171117150556.16947-5-eblake@redhat.com> In-Reply-To: <20171117150556.16947-1-eblake@redhat.com> References: <20171117150556.16947-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 17 Nov 2017 15:06:05 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 4/4] nbd/server: Fix error reporting for bad requests 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 , "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" The NBD spec says an attempt to NBD_CMD_TRIM on a read-only export should fail with EPERM, as a trim has the potential to change disk contents, but we were relying on the block layer to catch that for us, which might not always give the right error (and even if it does, it does not let us pass back a sane message for structured replies). The NBD spec says an attempt to NBD_CMD_WRITE_ZEROES out of bounds should fail with ENOSPC, not EINVAL. Our check for u64 offset + u32 length wraparound up front is pointless; nothing uses offset until after the second round of sanity checks, and we can just as easily ensure there is no wraparound by checking whether offset is in bounds (since a disk size cannot exceed off_t which is 63 bits, adding a 32-bit number for a valid offset can't overflow). Bonus: dropping the up-front check lets us keep the connection alive after NBD_CMD_WRITE, whereas before we would drop the connection (of course, any client sending a packet that would trigger the failure is already buggy, so it's also okay to drop the connection, but better quality-of-implementation never hurts). Solve all of these issues by some code motion and improved request validation. Signed-off-by: Eric Blake Message-Id: <20171115213557.3548-1-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- nbd/server.c | 36 ++++++++++++------------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index df771fd42f..7d6801b427 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1366,15 +1366,6 @@ static int nbd_co_receive_request(NBDRequestData *re= q, NBDRequest *request, return -EIO; } - /* Check for sanity in the parameters, part 1. Defer as many - * checks as possible until after reading any NBD_CMD_WRITE - * payload, so we can try and keep the connection alive. */ - if ((request->from + request->len) < request->from) { - error_setg(errp, - "integer overflow detected, you're probably being attac= ked"); - return -EINVAL; - } - if (request->type =3D=3D NBD_CMD_READ || request->type =3D=3D NBD_CMD_= WRITE) { if (request->len > NBD_MAX_BUFFER_SIZE) { error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u= )", @@ -1399,12 +1390,21 @@ static int nbd_co_receive_request(NBDRequestData *r= eq, NBDRequest *request, request->len); } - /* Sanity checks, part 2. */ - if (request->from + request->len > client->exp->size) { + /* Sanity checks. */ + if (client->exp->nbdflags & NBD_FLAG_READ_ONLY && + (request->type =3D=3D NBD_CMD_WRITE || + request->type =3D=3D NBD_CMD_WRITE_ZEROES || + request->type =3D=3D NBD_CMD_TRIM)) { + error_setg(errp, "Export is read-only"); + return -EROFS; + } + if (request->from > client->exp->size || + request->from + request->len > client->exp->size) { error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" P= RIu32 ", Size: %" PRIu64, request->from, request->len, (uint64_t)client->exp->size); - return request->type =3D=3D NBD_CMD_WRITE ? -ENOSPC : -EINVAL; + return (request->type =3D=3D NBD_CMD_WRITE || + request->type =3D=3D NBD_CMD_WRITE_ZEROES) ? -ENOSPC : -EI= NVAL; } valid_flags =3D NBD_CMD_FLAG_FUA; if (request->type =3D=3D NBD_CMD_READ && client->structured_reply) { @@ -1482,12 +1482,6 @@ static coroutine_fn void nbd_trip(void *opaque) break; case NBD_CMD_WRITE: - if (exp->nbdflags & NBD_FLAG_READ_ONLY) { - error_setg(&local_err, "Export is read-only"); - ret =3D -EROFS; - break; - } - flags =3D 0; if (request.flags & NBD_CMD_FLAG_FUA) { flags |=3D BDRV_REQ_FUA; @@ -1500,12 +1494,6 @@ static coroutine_fn void nbd_trip(void *opaque) break; case NBD_CMD_WRITE_ZEROES: - if (exp->nbdflags & NBD_FLAG_READ_ONLY) { - error_setg(&local_err, "Export is read-only"); - ret =3D -EROFS; - break; - } - flags =3D 0; if (request.flags & NBD_CMD_FLAG_FUA) { flags |=3D BDRV_REQ_FUA; --=20 2.13.6