From patchwork Mon Nov 5 16:37:20 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 150216 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp2829203ljp; Mon, 5 Nov 2018 08:47:34 -0800 (PST) X-Google-Smtp-Source: AJdET5coJfGmhLwQmXKWEne5G+GdKPU7v1KSIUfbCa8rrETFK0fa7gKbQCxGVCUGdBSPrthjWVDp X-Received: by 2002:a0c:8264:: with SMTP id h91mr22490826qva.116.1541436454766; Mon, 05 Nov 2018 08:47:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541436454; cv=none; d=google.com; s=arc-20160816; b=v671uIFcjnBT1kQw6t8vk1x7q0NTZDQftaUbpQRyujr+VL/XsXVwwasp8jw+IyQXyY a01dhwJpITcxONpkyaropTT5naracarc9ZosGaDRv0Pk9UE+H+cIy767YzjEGJDJmp/5 5+rHMBUZ3ppQoCM3mXO1yIeCiNYqSG1u1E4vZ8uySGjV650GWEa1ubHf7YpJHJ1nbHcx uZAdEqdrKMSb+sGkc0xCYkWofRtBG7Mm9iVpzxC0uJgtfmAbaA/TuELRcMdygxoF6TZC 7ZgtXA9skl5ZOPvg4FXNx+budNKWiUhzwtnHaPBh2k/NUdz3O8pL1gwhEeGwAFhwKM0j 9hzA== 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 :content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:to:from; bh=zZSVuIVWpwKlA21JM1L9J6NePRHiQeLP1fk4SB4mrl0=; b=lC8lSIuYxsn07fiICFpLvlT+DlTHOBDHgwPWWGuSmsdOJcqzvOUTVaSr2ARzYRh+kI eznETeZ6TosxPKuLyo1WAMvwXcnJHMihBTqJ2r72Gc9g4UjRZYkrBXGk6shU98bLTdIM uEUgY+As339DLeE4o0KpqfpFht9n6Hoj425B1Jb6EtP9lX+/AOZIvP6KyXKu8TKxVbWE 0ROwKs8Vb++TMfOTyzZGRRzgok1LAJJCeSKrHwR9Ge0aGOBk7c7jmMGbl+tgHD1wAvNv U+s4Rza3ZWuG6Xe1UYyj2cxGuSXw1NHzX6tpsplIYlmVvkcoGb9SlD+ZRZXg2MPF3mIV RCoQ== 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 17si7956355qvj.9.2018.11.05.08.47.34 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 05 Nov 2018 08:47:34 -0800 (PST) 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]:36216 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJi2I-0002XW-6I for patch@linaro.org; Mon, 05 Nov 2018 11:47:34 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50694) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gJhuG-0007wP-Sj for qemu-devel@nongnu.org; Mon, 05 Nov 2018 11:39:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gJhuB-0005T6-Jx for qemu-devel@nongnu.org; Mon, 05 Nov 2018 11:39:16 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51374) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gJhto-0003yq-Qr; Mon, 05 Nov 2018 11:38:49 -0500 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 9A8BC8666F; Mon, 5 Nov 2018 16:38:31 +0000 (UTC) Received: from linux.fritz.box.com (ovpn-117-198.ams2.redhat.com [10.36.117.198]) by smtp.corp.redhat.com (Postfix) with ESMTP id 591C160C46; Mon, 5 Nov 2018 16:38:28 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Mon, 5 Nov 2018 17:37:20 +0100 Message-Id: <20181105163744.25139-13-kwolf@redhat.com> In-Reply-To: <20181105163744.25139-1-kwolf@redhat.com> References: <20181105163744.25139-1-kwolf@redhat.com> MIME-Version: 1.0 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.26]); Mon, 05 Nov 2018 16:38:31 +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 12/36] block/vhdx: 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: kwolf@redhat.com, peter.maydell@linaro.org, qemu-devel@nongnu.org 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. There are a few places where the in-place swap function is used on something other than a packed struct field; we convert those anyway, for consistency. Patch produced with scripts/coccinelle/inplace-byteswaps.cocci. Signed-off-by: Peter Maydell Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf --- block/vhdx.h | 12 ++--- block/vhdx-endian.c | 118 ++++++++++++++++++++++---------------------- block/vhdx-log.c | 4 +- block/vhdx.c | 18 +++---- 4 files changed, 76 insertions(+), 76 deletions(-) -- 2.19.1 diff --git a/block/vhdx.h b/block/vhdx.h index 7003ab7a79..3a5f5293ad 100644 --- a/block/vhdx.h +++ b/block/vhdx.h @@ -420,16 +420,16 @@ int vhdx_log_write_and_flush(BlockDriverState *bs, BDRVVHDXState *s, static inline void leguid_to_cpus(MSGUID *guid) { - le32_to_cpus(&guid->data1); - le16_to_cpus(&guid->data2); - le16_to_cpus(&guid->data3); + guid->data1 = le32_to_cpu(guid->data1); + guid->data2 = le16_to_cpu(guid->data2); + guid->data3 = le16_to_cpu(guid->data3); } static inline void cpu_to_leguids(MSGUID *guid) { - cpu_to_le32s(&guid->data1); - cpu_to_le16s(&guid->data2); - cpu_to_le16s(&guid->data3); + guid->data1 = cpu_to_le32(guid->data1); + guid->data2 = cpu_to_le16(guid->data2); + guid->data3 = cpu_to_le16(guid->data3); } void vhdx_header_le_import(VHDXHeader *h); diff --git a/block/vhdx-endian.c b/block/vhdx-endian.c index 41fbdd2b8f..ebfa33cb8a 100644 --- a/block/vhdx-endian.c +++ b/block/vhdx-endian.c @@ -35,18 +35,18 @@ void vhdx_header_le_import(VHDXHeader *h) { assert(h != NULL); - le32_to_cpus(&h->signature); - le32_to_cpus(&h->checksum); - le64_to_cpus(&h->sequence_number); + h->signature = le32_to_cpu(h->signature); + h->checksum = le32_to_cpu(h->checksum); + h->sequence_number = le64_to_cpu(h->sequence_number); leguid_to_cpus(&h->file_write_guid); leguid_to_cpus(&h->data_write_guid); leguid_to_cpus(&h->log_guid); - le16_to_cpus(&h->log_version); - le16_to_cpus(&h->version); - le32_to_cpus(&h->log_length); - le64_to_cpus(&h->log_offset); + h->log_version = le16_to_cpu(h->log_version); + h->version = le16_to_cpu(h->version); + h->log_length = le32_to_cpu(h->log_length); + h->log_offset = le64_to_cpu(h->log_offset); } void vhdx_header_le_export(VHDXHeader *orig_h, VHDXHeader *new_h) @@ -80,68 +80,68 @@ void vhdx_log_desc_le_import(VHDXLogDescriptor *d) { assert(d != NULL); - le32_to_cpus(&d->signature); - le64_to_cpus(&d->file_offset); - le64_to_cpus(&d->sequence_number); + d->signature = le32_to_cpu(d->signature); + d->file_offset = le64_to_cpu(d->file_offset); + d->sequence_number = le64_to_cpu(d->sequence_number); } void vhdx_log_desc_le_export(VHDXLogDescriptor *d) { assert(d != NULL); - cpu_to_le32s(&d->signature); - cpu_to_le32s(&d->trailing_bytes); - cpu_to_le64s(&d->leading_bytes); - cpu_to_le64s(&d->file_offset); - cpu_to_le64s(&d->sequence_number); + d->signature = cpu_to_le32(d->signature); + d->trailing_bytes = cpu_to_le32(d->trailing_bytes); + d->leading_bytes = cpu_to_le64(d->leading_bytes); + d->file_offset = cpu_to_le64(d->file_offset); + d->sequence_number = cpu_to_le64(d->sequence_number); } void vhdx_log_data_le_import(VHDXLogDataSector *d) { assert(d != NULL); - le32_to_cpus(&d->data_signature); - le32_to_cpus(&d->sequence_high); - le32_to_cpus(&d->sequence_low); + d->data_signature = le32_to_cpu(d->data_signature); + d->sequence_high = le32_to_cpu(d->sequence_high); + d->sequence_low = le32_to_cpu(d->sequence_low); } void vhdx_log_data_le_export(VHDXLogDataSector *d) { assert(d != NULL); - cpu_to_le32s(&d->data_signature); - cpu_to_le32s(&d->sequence_high); - cpu_to_le32s(&d->sequence_low); + d->data_signature = cpu_to_le32(d->data_signature); + d->sequence_high = cpu_to_le32(d->sequence_high); + d->sequence_low = cpu_to_le32(d->sequence_low); } void vhdx_log_entry_hdr_le_import(VHDXLogEntryHeader *hdr) { assert(hdr != NULL); - le32_to_cpus(&hdr->signature); - le32_to_cpus(&hdr->checksum); - le32_to_cpus(&hdr->entry_length); - le32_to_cpus(&hdr->tail); - le64_to_cpus(&hdr->sequence_number); - le32_to_cpus(&hdr->descriptor_count); + hdr->signature = le32_to_cpu(hdr->signature); + hdr->checksum = le32_to_cpu(hdr->checksum); + hdr->entry_length = le32_to_cpu(hdr->entry_length); + hdr->tail = le32_to_cpu(hdr->tail); + hdr->sequence_number = le64_to_cpu(hdr->sequence_number); + hdr->descriptor_count = le32_to_cpu(hdr->descriptor_count); leguid_to_cpus(&hdr->log_guid); - le64_to_cpus(&hdr->flushed_file_offset); - le64_to_cpus(&hdr->last_file_offset); + hdr->flushed_file_offset = le64_to_cpu(hdr->flushed_file_offset); + hdr->last_file_offset = le64_to_cpu(hdr->last_file_offset); } void vhdx_log_entry_hdr_le_export(VHDXLogEntryHeader *hdr) { assert(hdr != NULL); - cpu_to_le32s(&hdr->signature); - cpu_to_le32s(&hdr->checksum); - cpu_to_le32s(&hdr->entry_length); - cpu_to_le32s(&hdr->tail); - cpu_to_le64s(&hdr->sequence_number); - cpu_to_le32s(&hdr->descriptor_count); + hdr->signature = cpu_to_le32(hdr->signature); + hdr->checksum = cpu_to_le32(hdr->checksum); + hdr->entry_length = cpu_to_le32(hdr->entry_length); + hdr->tail = cpu_to_le32(hdr->tail); + hdr->sequence_number = cpu_to_le64(hdr->sequence_number); + hdr->descriptor_count = cpu_to_le32(hdr->descriptor_count); cpu_to_leguids(&hdr->log_guid); - cpu_to_le64s(&hdr->flushed_file_offset); - cpu_to_le64s(&hdr->last_file_offset); + hdr->flushed_file_offset = cpu_to_le64(hdr->flushed_file_offset); + hdr->last_file_offset = cpu_to_le64(hdr->last_file_offset); } @@ -150,18 +150,18 @@ void vhdx_region_header_le_import(VHDXRegionTableHeader *hdr) { assert(hdr != NULL); - le32_to_cpus(&hdr->signature); - le32_to_cpus(&hdr->checksum); - le32_to_cpus(&hdr->entry_count); + hdr->signature = le32_to_cpu(hdr->signature); + hdr->checksum = le32_to_cpu(hdr->checksum); + hdr->entry_count = le32_to_cpu(hdr->entry_count); } void vhdx_region_header_le_export(VHDXRegionTableHeader *hdr) { assert(hdr != NULL); - cpu_to_le32s(&hdr->signature); - cpu_to_le32s(&hdr->checksum); - cpu_to_le32s(&hdr->entry_count); + hdr->signature = cpu_to_le32(hdr->signature); + hdr->checksum = cpu_to_le32(hdr->checksum); + hdr->entry_count = cpu_to_le32(hdr->entry_count); } void vhdx_region_entry_le_import(VHDXRegionTableEntry *e) @@ -169,9 +169,9 @@ void vhdx_region_entry_le_import(VHDXRegionTableEntry *e) assert(e != NULL); leguid_to_cpus(&e->guid); - le64_to_cpus(&e->file_offset); - le32_to_cpus(&e->length); - le32_to_cpus(&e->data_bits); + e->file_offset = le64_to_cpu(e->file_offset); + e->length = le32_to_cpu(e->length); + e->data_bits = le32_to_cpu(e->data_bits); } void vhdx_region_entry_le_export(VHDXRegionTableEntry *e) @@ -179,9 +179,9 @@ void vhdx_region_entry_le_export(VHDXRegionTableEntry *e) assert(e != NULL); cpu_to_leguids(&e->guid); - cpu_to_le64s(&e->file_offset); - cpu_to_le32s(&e->length); - cpu_to_le32s(&e->data_bits); + e->file_offset = cpu_to_le64(e->file_offset); + e->length = cpu_to_le32(e->length); + e->data_bits = cpu_to_le32(e->data_bits); } @@ -190,16 +190,16 @@ void vhdx_metadata_header_le_import(VHDXMetadataTableHeader *hdr) { assert(hdr != NULL); - le64_to_cpus(&hdr->signature); - le16_to_cpus(&hdr->entry_count); + hdr->signature = le64_to_cpu(hdr->signature); + hdr->entry_count = le16_to_cpu(hdr->entry_count); } void vhdx_metadata_header_le_export(VHDXMetadataTableHeader *hdr) { assert(hdr != NULL); - cpu_to_le64s(&hdr->signature); - cpu_to_le16s(&hdr->entry_count); + hdr->signature = cpu_to_le64(hdr->signature); + hdr->entry_count = cpu_to_le16(hdr->entry_count); } void vhdx_metadata_entry_le_import(VHDXMetadataTableEntry *e) @@ -207,16 +207,16 @@ void vhdx_metadata_entry_le_import(VHDXMetadataTableEntry *e) assert(e != NULL); leguid_to_cpus(&e->item_id); - le32_to_cpus(&e->offset); - le32_to_cpus(&e->length); - le32_to_cpus(&e->data_bits); + e->offset = le32_to_cpu(e->offset); + e->length = le32_to_cpu(e->length); + e->data_bits = le32_to_cpu(e->data_bits); } void vhdx_metadata_entry_le_export(VHDXMetadataTableEntry *e) { assert(e != NULL); cpu_to_leguids(&e->item_id); - cpu_to_le32s(&e->offset); - cpu_to_le32s(&e->length); - cpu_to_le32s(&e->data_bits); + e->offset = cpu_to_le32(e->offset); + e->length = cpu_to_le32(e->length); + e->data_bits = cpu_to_le32(e->data_bits); } diff --git a/block/vhdx-log.c b/block/vhdx-log.c index d2f1b98199..ecd64266c5 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -835,11 +835,11 @@ static void vhdx_log_raw_to_le_sector(VHDXLogDescriptor *desc, /* 8 + 4084 + 4 = 4096, 1 log sector */ memcpy(&desc->leading_bytes, data, 8); data += 8; - cpu_to_le64s(&desc->leading_bytes); + desc->leading_bytes = cpu_to_le64(desc->leading_bytes); memcpy(sector->data, data, 4084); data += 4084; memcpy(&desc->trailing_bytes, data, 4); - cpu_to_le32s(&desc->trailing_bytes); + desc->trailing_bytes = cpu_to_le32(desc->trailing_bytes); data += 4; sector->sequence_high = (uint32_t) (seq >> 32); diff --git a/block/vhdx.c b/block/vhdx.c index 0795ca1985..b785aef4b7 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -156,7 +156,7 @@ uint32_t vhdx_update_checksum(uint8_t *buf, size_t size, int crc_offset) memset(buf + crc_offset, 0, sizeof(crc)); crc = crc32c(0xffffffff, buf, size); - cpu_to_le32s(&crc); + crc = cpu_to_le32(crc); memcpy(buf + crc_offset, &crc, sizeof(crc)); return crc; @@ -753,8 +753,8 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s) goto exit; } - le32_to_cpus(&s->params.block_size); - le32_to_cpus(&s->params.data_bits); + s->params.block_size = le32_to_cpu(s->params.block_size); + s->params.data_bits = le32_to_cpu(s->params.data_bits); /* We now have the file parameters, so we can tell if this is a @@ -803,9 +803,9 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s) goto exit; } - le64_to_cpus(&s->virtual_disk_size); - le32_to_cpus(&s->logical_sector_size); - le32_to_cpus(&s->physical_sector_size); + s->virtual_disk_size = le64_to_cpu(s->virtual_disk_size); + s->logical_sector_size = le32_to_cpu(s->logical_sector_size); + s->physical_sector_size = le32_to_cpu(s->physical_sector_size); if (s->params.block_size < VHDX_BLOCK_SIZE_MIN || s->params.block_size > VHDX_BLOCK_SIZE_MAX) { @@ -985,7 +985,7 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags, /* endian convert, and verify populated BAT field file offsets against * region table and log entries */ for (i = 0; i < s->bat_entries; i++) { - le64_to_cpus(&s->bat[i]); + s->bat[i] = le64_to_cpu(s->bat[i]); if (payblocks--) { /* payload bat entries */ if ((s->bat[i] & VHDX_BAT_STATE_BIT_MASK) == @@ -1509,7 +1509,7 @@ static int vhdx_create_new_metadata(BlockBackend *blk, mt_file_params->block_size = cpu_to_le32(block_size); if (type == VHDX_TYPE_FIXED) { mt_file_params->data_bits |= VHDX_PARAMS_LEAVE_BLOCKS_ALLOCED; - cpu_to_le32s(&mt_file_params->data_bits); + mt_file_params->data_bits = cpu_to_le32(mt_file_params->data_bits); } vhdx_guid_generate(&mt_page83->page_83_data); @@ -1656,7 +1656,7 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, sinfo.file_offset = ROUND_UP(sinfo.file_offset, MiB); vhdx_update_bat_table_entry(blk_bs(blk), s, &sinfo, &unused, &unused, block_state); - cpu_to_le64s(&s->bat[sinfo.bat_idx]); + s->bat[sinfo.bat_idx] = cpu_to_le64(s->bat[sinfo.bat_idx]); sector_num += s->sectors_per_block; } ret = blk_pwrite(blk, file_offset, s->bat, length, 0);