From nobody Wed Nov 5 22:23:54 2025 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; dmarc=fail(p=none dis=none) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 153806659926797.67090038924209; Thu, 27 Sep 2018 09:43:19 -0700 (PDT) Received: from localhost ([::1]:37981 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5ZNh-0005ml-7T for importer@patchew.org; Thu, 27 Sep 2018 12:43:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g5ZMd-0004of-7O for qemu-devel@nongnu.org; Thu, 27 Sep 2018 12:42:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g5ZMc-00008u-0F for qemu-devel@nongnu.org; Thu, 27 Sep 2018 12:42:07 -0400 Received: from orth.archaic.org.uk ([2001:8b0:1d0::2]:48686) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1g5ZMb-00008M-N6; Thu, 27 Sep 2018 12:42:05 -0400 Received: from pm215 by orth.archaic.org.uk with local (Exim 4.89) (envelope-from ) id 1g5ZMZ-0002yE-CI; Thu, 27 Sep 2018 17:42:03 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Date: Thu, 27 Sep 2018 17:42:00 +0100 Message-Id: <20180927164200.15097-1-peter.maydell@linaro.org> X-Mailer: git-send-email 2.19.0 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:8b0:1d0::2 Subject: [Qemu-devel] [PATCH] 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: Paolo Bonzini , qemu-block@nongnu.org, patches@linaro.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RDMRC_1 RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" 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 =3D be16_to_cpu(E); @@ expression E; @@ -be32_to_cpus(&E); +E =3D be32_to_cpu(E); @@ expression E; @@ -be64_to_cpus(&E); +E =3D be64_to_cpu(E); @@ expression E; @@ -cpu_to_be16s(&E); +E =3D cpu_to_be16(E); @@ expression E; @@ -cpu_to_be32s(&E); +E =3D cpu_to_be32(E); @@ expression E; @@ -cpu_to_be64s(&E); +E =3D cpu_to_be64(E); Signed-off-by: Peter Maydell Reviewed-by: Eric Blake Reviewed-by: Philippe Mathieu-Daud=C3=A9 --- Disclaimer: tested only with "make check", but it is a mechanically generated patch... nbd/client.c | 44 ++++++++++++++++++++++---------------------- nbd/server.c | 16 ++++++++-------- 2 files changed, 30 insertions(+), 30 deletions(-) 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 =3D be64_to_cpu(reply->magic); + reply->option =3D be32_to_cpu(reply->option); + reply->type =3D be32_to_cpu(reply->type); + reply->length =3D be32_to_cpu(reply->length); =20 trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->op= tion), reply->type, nbd_rep_lookup(reply->type= ), @@ -396,7 +396,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, return -1; } len -=3D sizeof(type); - be16_to_cpus(&type); + type =3D be16_to_cpu(type); switch (type) { case NBD_INFO_EXPORT: if (len !=3D sizeof(info->size) + sizeof(info->flags)) { @@ -410,13 +410,13 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wa= ntname, nbd_send_opt_abort(ioc); return -1; } - be64_to_cpus(&info->size); + info->size =3D 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 =3D be16_to_cpu(info->flags); trace_nbd_receive_negotiate_size_flags(info->size, info->flags= ); break; =20 @@ -433,7 +433,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->min_block); + info->min_block =3D 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 *want= name, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->opt_block); + info->opt_block =3D 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 *want= name, nbd_send_opt_abort(ioc); return -1; } - be32_to_cpus(&info->max_block); + info->max_block =3D 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 =3D be32_to_cpu(received_id); =20 reply.length -=3D sizeof(received_id); name =3D 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 =3D be64_to_cpu(info->size); =20 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 =3D be16_to_cpu(info->flags); } else if (magic =3D=3D NBD_CLIENT_MAGIC) { uint32_t oldflags; =20 @@ -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 =3D be64_to_cpu(info->size); =20 if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) { error_prepend(errp, "Failed to read export flags: "); goto fail; } - be32_to_cpus(&oldflags); + oldflags =3D be32_to_cpu(oldflags); if (oldflags & ~0xffff) { error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflag= s); goto fail; @@ -1080,8 +1080,8 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, = NBDSimpleReply *reply, return ret; } =20 - be32_to_cpus(&reply->error); - be64_to_cpus(&reply->handle); + reply->error =3D be32_to_cpu(reply->error); + reply->handle =3D be64_to_cpu(reply->handle); =20 return 0; } @@ -1105,10 +1105,10 @@ static int nbd_receive_structured_reply_chunk(QIOCh= annel *ioc, return ret; } =20 - be16_to_cpus(&chunk->flags); - be16_to_cpus(&chunk->type); - be64_to_cpus(&chunk->handle); - be32_to_cpus(&chunk->length); + chunk->flags =3D be16_to_cpu(chunk->flags); + chunk->type =3D be16_to_cpu(chunk->type); + chunk->handle =3D be64_to_cpu(chunk->handle); + chunk->length =3D be32_to_cpu(chunk->length); =20 return 0; } @@ -1128,7 +1128,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *repl= y, Error **errp) return ret; } =20 - be32_to_cpus(&reply->magic); + reply->magic =3D be32_to_cpu(reply->magic); =20 switch (reply->magic) { case NBD_SIMPLE_REPLY_MAGIC: diff --git a/nbd/server.c b/nbd/server.c index ea5fe0eb336..6f4e710a496 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -333,7 +333,7 @@ static int nbd_opt_read_name(NBDClient *client, char *n= ame, uint32_t *length, if (ret <=3D 0) { return ret; } - cpu_to_be32s(&len); + len =3D cpu_to_be32(len); =20 if (len > NBD_MAX_NAME_SIZE) { return nbd_opt_invalid(client, errp, @@ -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] =3D 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] =3D cpu_to_be32(sizes[0]); + sizes[1] =3D cpu_to_be32(sizes[1]); + sizes[2] =3D cpu_to_be32(sizes[2]); rc =3D 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 <=3D 0) { return ret; } - cpu_to_be32s(&len); + len =3D cpu_to_be32(len); =20 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 <=3D 0) { return ret; } - cpu_to_be32s(&nb_queries); + nb_queries =3D cpu_to_be32(nb_queries); trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt), export_name, nb_queries); =20 @@ -1049,7 +1049,7 @@ static int nbd_negotiate_options(NBDClient *client, u= int16_t myflags, error_prepend(errp, "read failed: "); return -EIO; } - be32_to_cpus(&flags); + flags =3D be32_to_cpu(flags); trace_nbd_negotiate_options_flags(flags); if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) { fixedNewstyle =3D true; @@ -1873,7 +1873,7 @@ static int blockstatus_to_extent_be(BlockDriverState = *bs, uint64_t offset, remaining_bytes -=3D num; } =20 - cpu_to_be32s(&extent->flags); + extent->flags =3D cpu_to_be32(extent->flags); extent->length =3D cpu_to_be32(bytes - remaining_bytes); =20 return 0; --=20 2.19.0