From nobody Tue May 7 02:15:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.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 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1553738632632682.6956549590485; Wed, 27 Mar 2019 19:03:52 -0700 (PDT) Received: from localhost ([127.0.0.1]:56546 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9KOR-00042b-4G for importer@patchew.org; Wed, 27 Mar 2019 22:03:47 -0400 Received: from eggs.gnu.org ([209.51.188.92]:52210) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h9KNY-0003fM-3T for qemu-devel@nongnu.org; Wed, 27 Mar 2019 22:02:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h9KNW-0002FV-VJ for qemu-devel@nongnu.org; Wed, 27 Mar 2019 22:02:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44118) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h9KNU-0002EM-7B; Wed, 27 Mar 2019 22:02:48 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BD6EA309E976; Thu, 28 Mar 2019 02:02:44 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-59.phx2.redhat.com [10.3.116.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id C0AF719C5A; Thu, 28 Mar 2019 02:02:40 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 27 Mar 2019 21:02:37 -0500 Message-Id: <20190328020237.6853-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Thu, 28 Mar 2019 02:02:44 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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.0?] nbd/client: Deal with unaligned size 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: Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, rjones@redhat.com, Max Reitz , jsnow@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" When a server advertises an unaligned size but no block sizes, the code was rounding up to a sector-aligned size (a known limitation of bdrv_getlength(); maybe 4.1 will fix it to be byte-accurate), then assuming a request_alignment of 512 (the recommendation of the NBD spec for maximum portability). However, this means that qemu will actually attempt to access the padding bytes of the trailing partial sector, without realizing that it is risky. An easy demonstration, using nbdkit as the server: $ nbdkit -fv random size=3D1023 $ qemu-io -r -f raw -c 'r -v 0 1023' nbd://localhost:10809 read failed: Invalid argument because the client rounded the request up to 1024 bytes, which nbdkit then rejected as beyond the advertised size of 1023. Note that qemu as the server refuses to send an unaligned size, as it has already rounded the unaligned image up to sector size, and then happily resizes the image on access (at least when serving a POSIX file over NBD). But since third-party servers may decide to kill the connection when we access out of bounds, it's better to just ignore the unaligned tail than it is to risk problems. We can undo this hack later once the block layer quits rounding sizes inappropriately. Reported-by: Richard W.M. Jones Signed-off-by: Eric Blake --- This is the minimal patch to avoid our client behaving out-of-spec, but I don't like that it makes the tail of the server's file unreadable. A better solution of auditing the block layer to use a proper byte length everywhere instead of rounding up may be 4.1 material, but is certainly not appropriate for 4.0-rc2. I could also teach the NBD code to compare every request from the block length against client.info.size, and manually handle the tail itself, but that feels like a lot of pain (for something the block layer should be able to do generically, and where any NBD-specific tail-handling code just slows down the common case and would be ripped out again once the block layer quits rounding up). I'm willing to include this with my other NBD patches slated for -rc2 as part of my suite of improved third-party interoperability, but only if we agree that the truncation is appropriate. Note that nbdkit includes '--filter=3Dtruncate round-up=3D512' as a way to expose the unaligned tail without making qemu trigger out-of-bounds operations: reads of the tail now succeed with contents of 0; writes fail with ENOSPC unless the contents requested to be written into the tail are all 0s. So not fixing the bug for 4.0, and telling people to use nbdkit's truncate filter instead, is also a viable option. block/nbd.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/block/nbd.c b/block/nbd.c index 2e72df528ac..a2cd365f646 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -463,7 +463,17 @@ static int64_t nbd_getlength(BlockDriverState *bs) { BDRVNBDState *s =3D bs->opaque; - return s->client.info.size; + /* + * HACK: Round down to sector alignment. qemu as server always + * advertises a sector-aligned image, so we only ever see + * unaligned sizes with third-party servers. The block layer + * defaults to rounding up, but that can result in us attempting + * to read beyond EOF from the remote server; better is to + * forcefully round down here and just treat the last few bytes + * from the server as unreadable. This can all go away once the + * block layer uses actual byte lengths instead of rounding up. + */ + return QEMU_ALIGN_DOWN(s->client.info.size, BDRV_SECTOR_SIZE); } static void nbd_detach_aio_context(BlockDriverState *bs) --=20 2.20.1