From patchwork Wed Oct 3 21:22:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 148085 Delivered-To: patch@linaro.org Received: by 2002:a2e:8595:0:0:0:0:0 with SMTP id b21-v6csp150467lji; Wed, 3 Oct 2018 14:26:05 -0700 (PDT) X-Google-Smtp-Source: ACcGV61hjRipWX1k4pyADPu5zRZFBUHBEHSVyJV145pxOkkQA12DQXbbsx2V78+BnzmcytY9IVy4 X-Received: by 2002:aed:22a6:: with SMTP id p35-v6mr2950712qtc.48.1538601965006; Wed, 03 Oct 2018 14:26:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1538601965; cv=none; d=google.com; s=arc-20160816; b=le//RmJ/MgCWqhZNHl2cs+f+426NjHcaDVq3/+0GneILzBSp/90HFEFGgwbWKozj4r nsZm7/RjHMDCxI1DCp0iYRYPI7PLX5ETE5fC6rzMhwE+t41I8B22JZAxivw2UjyHJ0ws /DXPQBgEbizI9JCG3wYl4PaPr2yhKtSuUZUItShK7zHscbzp7WDQ58SxHGSyMyD3sgCG T99LOEWrlT5oMqOErAL3x6mzjdYrJbwdqj7IeEwE3GEeuGSLvJyZoI5Rx8JNlK86aTt0 RUFg12N2zz98Jddyw5rVv33A2as53TN+dLuKfyeYLPiOjcMXhHFYOFF9xemQf3IwhNPb jhfw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject:references:in-reply-to :message-id:date:to:from; bh=Gb2t/8z7b+kKMSAsnxwafKpp/dnm+EqTNJTfnrpoyJU=; b=WnBdHpeuoYVhroZeBwbpcqvVvyZOxfxMl8Zcst9Xi4mJSpXd6peuASYvLsiaOMpRvV /nXplVx3U7N+lYNBFiuZLDahBmoqgp5xZcfpvzffzeLQAZZcTHy8xQr9WRkp7tQx4gTi pBcP963lDrXS+8Dj0a78zgyWt6m8+6d4A2kyZ/+Tf59BAhH8ZntnttPvX447tzI9UOVq +pD9hSE8e/FKgFJ5RsQchThEqUDe3I1GnIQ0riw6SRjfh+RGSgZiPtGWoJJyX4DV324r xgH/SM1CxzxKdI6Lu2S3rXXAgMODNwU34qghpynkDgl/Eef/iLkZU0QMgEnI9QGY/D/1 SrlA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id x6-v6si1711397qvb.123.2018.10.03.14.26.04 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 03 Oct 2018 14:26:04 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:53238 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g7oei-0004Fy-G2 for patch@linaro.org; Wed, 03 Oct 2018 17:26:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46349) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g7obO-0001fG-1p for qemu-devel@nongnu.org; Wed, 03 Oct 2018 17:22:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g7obM-000110-3B for qemu-devel@nongnu.org; Wed, 03 Oct 2018 17:22:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36238) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1g7obF-0000qL-TA; Wed, 03 Oct 2018 17:22:30 -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 8E2E43003B32; Wed, 3 Oct 2018 21:22:17 +0000 (UTC) Received: from red.redhat.com (ovpn-126-73.rdu2.redhat.com [10.10.126.73]) by smtp.corp.redhat.com (Postfix) with ESMTP id 428D360F97; Wed, 3 Oct 2018 21:22:16 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 3 Oct 2018 16:22:07 -0500 Message-Id: <20181003212211.812374-2-eblake@redhat.com> In-Reply-To: <20181003212211.812374-1-eblake@redhat.com> References: <20181003212211.812374-1-eblake@redhat.com> 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.46]); Wed, 03 Oct 2018 21:22:17 +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] [PULL 1/5] nbd: Don't take address of fields in packed structs 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: Peter Maydell , "open list:Network Block Dev..." , Paolo Bonzini Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Peter Maydell Taking the address of a field in a packed struct is a bad idea, because it might not be actually aligned enough for that pointer type (and thus cause a crash on dereference on some host architectures). Newer versions of clang warn about this. Avoid the bug by not using the "modify in place" byte swapping functions. This patch was produced with the following spatch script: @@ expression E; @@ -be16_to_cpus(&E); +E = be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E = be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E = be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E = cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E = cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E = cpu_to_be64(E); Signed-off-by: Peter Maydell Message-Id: <20180927164200.15097-1-peter.maydell@linaro.org> Reviewed-by: Eric Blake [eblake: rebase, and squash in missed changes] Signed-off-by: Eric Blake --- nbd/client.c | 44 ++++++++++++++++++++++---------------------- nbd/server.c | 24 ++++++++++++------------ 2 files changed, 34 insertions(+), 34 deletions(-) -- 2.17.1 diff --git a/nbd/client.c b/nbd/client.c index 40b74d9761f..b4d457a19ad 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -117,10 +117,10 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt, nbd_send_opt_abort(ioc); return -1; } - be64_to_cpus(&reply->magic); - be32_to_cpus(&reply->option); - be32_to_cpus(&reply->type); - be32_to_cpus(&reply->length); + reply->magic = be64_to_cpu(reply->magic); + reply->option = be32_to_cpu(reply->option); + reply->type = be32_to_cpu(reply->type); + reply->length = be32_to_cpu(reply->length); trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->option), reply->type, nbd_rep_lookup(reply->type), @@ -396,7 +396,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, return -1; } len -= sizeof(type); - be16_to_cpus(&type); + type = be16_to_cpu(type); switch (type) { case NBD_INFO_EXPORT: if (len != sizeof(info->size) + sizeof(info->flags)) { @@ -410,13 +410,13 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, nbd_send_opt_abort(ioc); return -1; } - be64_to_cpus(&info->size); + info->size = be64_to_cpu(info->size); if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) { error_prepend(errp, "failed to read info flags: "); nbd_send_opt_abort(ioc); return -1; } - be16_to_cpus(&info->flags); + info->flags = be16_to_cpu(info->flags); trace_nbd_receive_negotiate_size_flags(info->size, info->flags); break; @@ -433,7 +433,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->min_block); + info->min_block = be32_to_cpu(info->min_block); if (!is_power_of_2(info->min_block)) { error_setg(errp, "server minimum block size %" PRIu32 " is not a power of two", info->min_block); @@ -447,7 +447,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->opt_block); + info->opt_block = be32_to_cpu(info->opt_block); if (!is_power_of_2(info->opt_block) || info->opt_block < info->min_block) { error_setg(errp, "server preferred block size %" PRIu32 @@ -461,7 +461,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->max_block); + info->max_block = be32_to_cpu(info->max_block); if (info->max_block < info->min_block) { error_setg(errp, "server maximum block size %" PRIu32 " is not valid", info->max_block); @@ -668,7 +668,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) { return -1; } - be32_to_cpus(&received_id); + received_id = be32_to_cpu(received_id); reply.length -= sizeof(received_id); name = g_malloc(reply.length + 1); @@ -872,13 +872,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, error_prepend(errp, "Failed to read export length: "); goto fail; } - be64_to_cpus(&info->size); + info->size = be64_to_cpu(info->size); if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) { error_prepend(errp, "Failed to read export flags: "); goto fail; } - be16_to_cpus(&info->flags); + info->flags = be16_to_cpu(info->flags); } else if (magic == NBD_CLIENT_MAGIC) { uint32_t oldflags; @@ -895,13 +895,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, error_prepend(errp, "Failed to read export length: "); goto fail; } - be64_to_cpus(&info->size); + info->size = be64_to_cpu(info->size); if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) { error_prepend(errp, "Failed to read export flags: "); goto fail; } - be32_to_cpus(&oldflags); + oldflags = be32_to_cpu(oldflags); if (oldflags & ~0xffff) { error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags); goto fail; @@ -1080,8 +1080,8 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply, return ret; } - be32_to_cpus(&reply->error); - be64_to_cpus(&reply->handle); + reply->error = be32_to_cpu(reply->error); + reply->handle = be64_to_cpu(reply->handle); return 0; } @@ -1105,10 +1105,10 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc, return ret; } - be16_to_cpus(&chunk->flags); - be16_to_cpus(&chunk->type); - be64_to_cpus(&chunk->handle); - be32_to_cpus(&chunk->length); + chunk->flags = be16_to_cpu(chunk->flags); + chunk->type = be16_to_cpu(chunk->type); + chunk->handle = be64_to_cpu(chunk->handle); + chunk->length = be32_to_cpu(chunk->length); return 0; } @@ -1128,7 +1128,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp) return ret; } - be32_to_cpus(&reply->magic); + reply->magic = be32_to_cpu(reply->magic); switch (reply->magic) { case NBD_SIMPLE_REPLY_MAGIC: diff --git a/nbd/server.c b/nbd/server.c index c3dd402b45e..98d0fa25158 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -333,7 +333,7 @@ static int nbd_opt_read_name(NBDClient *client, char *name, uint32_t *length, if (ret <= 0) { return ret; } - cpu_to_be32s(&len); + len = cpu_to_be32(len); if (len > NBD_MAX_NAME_SIZE) { return nbd_opt_invalid(client, errp, @@ -486,7 +486,7 @@ static int nbd_negotiate_send_info(NBDClient *client, if (rc < 0) { return rc; } - cpu_to_be16s(&info); + info = cpu_to_be16(info); if (nbd_write(client->ioc, &info, sizeof(info), errp) < 0) { return -EIO; } @@ -551,14 +551,14 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags, if (rc <= 0) { return rc; } - be16_to_cpus(&requests); + requests = be16_to_cpu(requests); trace_nbd_negotiate_handle_info_requests(requests); while (requests--) { rc = nbd_opt_read(client, &request, sizeof(request), errp); if (rc <= 0) { return rc; } - be16_to_cpus(&request); + request = be16_to_cpu(request); trace_nbd_negotiate_handle_info_request(request, nbd_info_lookup(request)); /* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE; @@ -618,9 +618,9 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags, /* maximum - At most 32M, but smaller as appropriate. */ sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE); trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]); - cpu_to_be32s(&sizes[0]); - cpu_to_be32s(&sizes[1]); - cpu_to_be32s(&sizes[2]); + sizes[0] = cpu_to_be32(sizes[0]); + sizes[1] = cpu_to_be32(sizes[1]); + sizes[2] = cpu_to_be32(sizes[2]); rc = nbd_negotiate_send_info(client, NBD_INFO_BLOCK_SIZE, sizeof(sizes), sizes, errp); if (rc < 0) { @@ -904,7 +904,7 @@ static int nbd_negotiate_meta_query(NBDClient *client, if (ret <= 0) { return ret; } - cpu_to_be32s(&len); + len = cpu_to_be32(len); if (len < ns_len) { trace_nbd_negotiate_meta_query_skip("length too short"); @@ -971,7 +971,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, if (ret <= 0) { return ret; } - cpu_to_be32s(&nb_queries); + nb_queries = cpu_to_be32(nb_queries); trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt), export_name, nb_queries); @@ -1049,7 +1049,7 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags, error_prepend(errp, "read failed: "); return -EIO; } - be32_to_cpus(&flags); + flags = be32_to_cpu(flags); trace_nbd_negotiate_options_flags(flags); if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) { fixedNewstyle = true; @@ -1900,8 +1900,8 @@ static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset, extents_end = extent + 1; for (extent = extents; extent < extents_end; extent++) { - cpu_to_be32s(&extent->flags); - cpu_to_be32s(&extent->length); + extent->flags = cpu_to_be32(extent->flags); + extent->length = cpu_to_be32(extent->length); } *bytes -= remaining_bytes;