[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration/ram.c: Avoid taking address of fields
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH] migration/ram.c: Avoid taking address of fields in packed MultiFDInit_t struct |
Date: |
Wed, 26 Sep 2018 16:02:01 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
* Peter Maydell (address@hidden) wrote:
> 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:
>
> migration/ram.c:651:19: warning: taking address of packed member 'magic' of
> class or structure 'MultiFDInit_t' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> migration/ram.c:652:19: warning: taking address of packed member 'version' of
> class or structure 'MultiFDInit_t' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> migration/ram.c:737:19: warning: taking address of packed member 'magic' of
> class or structure 'MultiFDPacket_t' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> migration/ram.c:745:19: warning: taking address of packed member 'version' of
> class or structure 'MultiFDPacket_t' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
> migration/ram.c:755:19: warning: taking address of packed member 'size' of
> class or structure 'MultiFDPacket_t' may result in an unaligned pointer value
> [-Waddress-of-packed-member]
>
> Avoid the bug by not using the "modify in place" byteswapping
> functions.
>
> Signed-off-by: Peter Maydell <address@hidden>
Queued
> ---
> Now I have a machine with the version of clang that emits these warnings
> I'm starting to care a little more about them :-)
> ---
> migration/ram.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index f6fd8e5e096..4fc3bc36816 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -648,8 +648,8 @@ static int multifd_recv_initial_packet(QIOChannel *c,
> Error **errp)
> return -1;
> }
>
> - be32_to_cpus(&msg.magic);
> - be32_to_cpus(&msg.version);
> + msg.magic = be32_to_cpu(msg.magic);
> + msg.version = be32_to_cpu(msg.version);
>
> if (msg.magic != MULTIFD_MAGIC) {
> error_setg(errp, "multifd: received packet magic %x "
> @@ -734,7 +734,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
> *p, Error **errp)
> RAMBlock *block;
> int i;
>
> - be32_to_cpus(&packet->magic);
> + packet->magic = be32_to_cpu(packet->magic);
> if (packet->magic != MULTIFD_MAGIC) {
> error_setg(errp, "multifd: received packet "
> "magic %x and expected magic %x",
> @@ -742,7 +742,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
> *p, Error **errp)
> return -1;
> }
>
> - be32_to_cpus(&packet->version);
> + packet->version = be32_to_cpu(packet->version);
> if (packet->version != MULTIFD_VERSION) {
> error_setg(errp, "multifd: received packet "
> "version %d and expected version %d",
> @@ -752,7 +752,7 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
> *p, Error **errp)
>
> p->flags = be32_to_cpu(packet->flags);
>
> - be32_to_cpus(&packet->size);
> + packet->size = be32_to_cpu(packet->size);
> if (packet->size > migrate_multifd_page_count()) {
> error_setg(errp, "multifd: received packet "
> "with size %d and expected maximum size %d",
> --
> 2.19.0
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK