From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47996) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNrZj-0004nt-V8 for qemu-devel@nongnu.org; Wed, 12 Mar 2014 18:24:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WNrZb-0006yd-F1 for qemu-devel@nongnu.org; Wed, 12 Mar 2014 18:24:35 -0400 Received: from mail-qa0-x22a.google.com ([2607:f8b0:400d:c00::22a]:34321) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WNrZb-0006yW-AO for qemu-devel@nongnu.org; Wed, 12 Mar 2014 18:24:27 -0400 Received: by mail-qa0-f42.google.com with SMTP id k15so192486qaq.29 for ; Wed, 12 Mar 2014 15:24:26 -0700 (PDT) Sender: Richard Henderson Message-ID: <5320DE97.9030300@twiddle.net> Date: Wed, 12 Mar 2014 15:24:23 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1394480575-3698-1-git-send-email-mst@redhat.com> <1394480575-3698-2-git-send-email-mst@redhat.com> In-Reply-To: <1394480575-3698-2-git-send-email-mst@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] acpi-build: don't access unaligned addresses List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" , qemu-devel@nongnu.org Cc: Peter Maydell , Anthony Liguori On 03/10/2014 12:43 PM, Michael S. Tsirkin wrote: > casting an unaligned address to e.g. > uint32_t can trigger undefined behaviour in C. > Replace cast + assignment with memcpy. > > Reported-by: Peter Maydell > Signed-off-by: Michael S. Tsirkin > --- > hw/i386/acpi-build.c | 31 ++++++++++++++++--------------- > 1 file changed, 16 insertions(+), 15 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index b667d31..e08f514 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -466,9 +466,15 @@ static void acpi_align_size(GArray *blob, unsigned align) > g_array_set_size(blob, ROUND_UP(acpi_data_len(blob), align)); > } > > -/* Get pointer within table in a safe manner */ > -#define ACPI_BUILD_PTR(table, size, off, type) \ > - ((type *)(acpi_data_get_ptr(table, size, off, sizeof(type)))) > +/* Set a value within table in a safe manner */ > +#define ACPI_BUILD_SET_LE(table, size, off, bits, val) \ > + do { \ > + uint64_t ACPI_BUILD_SET_LE_val = le_bswap(val, bits); \ > + memcpy(acpi_data_get_ptr(table, size, off, \ > + (bits) / BITS_PER_BYTE), \ > + &ACPI_BUILD_SET_LE_val, \ > + (bits) / BITS_PER_BYTE); \ > + } while (0) > > static inline void *acpi_data_get_ptr(uint8_t *table_data, unsigned table_size, > unsigned off, unsigned size) > @@ -974,22 +980,17 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > static void patch_pci_windows(PcPciInfo *pci, uint8_t *start, unsigned size) > { > - *ACPI_BUILD_PTR(start, size, acpi_pci32_start[0], uint32_t) = > - cpu_to_le32(pci->w32.begin); > + ACPI_BUILD_SET_LE(start, size, acpi_pci32_start[0], 32, pci->w32.begin); You're working too hard at this because you're using the wrong bswap interface. Also, you missed one. I'll follow up with a patch I wrote just recently, but hadn't yet gotten around to posting. r~