* [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs
@ 2018-12-10 12:04 Peter Maydell
2018-12-10 12:35 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Peter Maydell @ 2018-12-10 12:04 UTC (permalink / raw)
To: qemu-devel
Cc: patches, qemu-s390x, Cornelia Huck, Halil Pasic,
Christian Borntraeger, Richard Henderson, David Hildenbrand
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.
Patch produced with scripts/coccinelle/inplace-byteswaps.cocci
(with a couple of long lines manually wrapped).
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
s390 also has some warnings in hw/s390x/css.c which are going to
be harder to fix, relating to taking the address of the 'pmcw' and
'scsw' fields in the SCHUB struct...
hw/s390x/virtio-ccw.c | 42 ++++++++++++++++++++++--------------------
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 212b3d3dead..c2b78c8e9b1 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -287,18 +287,18 @@ static int virtio_ccw_handle_set_vq(SubchDev *sch, CCW1 ccw, bool check_len,
}
if (is_legacy) {
ccw_dstream_read(&sch->cds, linfo);
- be64_to_cpus(&linfo.queue);
- be32_to_cpus(&linfo.align);
- be16_to_cpus(&linfo.index);
- be16_to_cpus(&linfo.num);
+ linfo.queue = be64_to_cpu(linfo.queue);
+ linfo.align = be32_to_cpu(linfo.align);
+ linfo.index = be16_to_cpu(linfo.index);
+ linfo.num = be16_to_cpu(linfo.num);
ret = virtio_ccw_set_vqs(sch, NULL, &linfo);
} else {
ccw_dstream_read(&sch->cds, info);
- be64_to_cpus(&info.desc);
- be16_to_cpus(&info.index);
- be16_to_cpus(&info.num);
- be64_to_cpus(&info.avail);
- be64_to_cpus(&info.used);
+ info.desc = be64_to_cpu(info.desc);
+ info.index = be16_to_cpu(info.index);
+ info.num = be16_to_cpu(info.num);
+ info.avail = be64_to_cpu(info.avail);
+ info.used = be64_to_cpu(info.used);
ret = virtio_ccw_set_vqs(sch, &info, NULL);
}
sch->curr_status.scsw.count = 0;
@@ -382,7 +382,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
features.features = 0;
}
ccw_dstream_rewind(&sch->cds);
- cpu_to_le32s(&features.features);
+ features.features = cpu_to_le32(features.features);
ccw_dstream_write(&sch->cds, features.features);
sch->curr_status.scsw.count = ccw.count - sizeof(features);
ret = 0;
@@ -403,7 +403,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
ret = -EFAULT;
} else {
ccw_dstream_read(&sch->cds, features);
- le32_to_cpus(&features.features);
+ features.features = le32_to_cpu(features.features);
if (features.index == 0) {
virtio_set_features(vdev,
(vdev->guest_features & 0xffffffff00000000ULL) |
@@ -546,7 +546,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
ret = -EFAULT;
} else {
ccw_dstream_read(&sch->cds, indicators);
- be64_to_cpus(&indicators);
+ indicators = be64_to_cpu(indicators);
dev->indicators = get_indicator(indicators, sizeof(uint64_t));
sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
ret = 0;
@@ -567,7 +567,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
ret = -EFAULT;
} else {
ccw_dstream_read(&sch->cds, indicators);
- be64_to_cpus(&indicators);
+ indicators = be64_to_cpu(indicators);
dev->indicators2 = get_indicator(indicators, sizeof(uint64_t));
sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
ret = 0;
@@ -588,14 +588,14 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
ret = -EFAULT;
} else {
ccw_dstream_read(&sch->cds, vq_config.index);
- be16_to_cpus(&vq_config.index);
+ vq_config.index = be16_to_cpu(vq_config.index);
if (vq_config.index >= VIRTIO_QUEUE_MAX) {
ret = -EINVAL;
break;
}
vq_config.num_max = virtio_queue_get_num(vdev,
vq_config.index);
- cpu_to_be16s(&vq_config.num_max);
+ vq_config.num_max = cpu_to_be16(vq_config.num_max);
ccw_dstream_write(&sch->cds, vq_config.num_max);
sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
ret = 0;
@@ -621,9 +621,11 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
if (ccw_dstream_read(&sch->cds, thinint)) {
ret = -EFAULT;
} else {
- be64_to_cpus(&thinint.ind_bit);
- be64_to_cpus(&thinint.summary_indicator);
- be64_to_cpus(&thinint.device_indicator);
+ thinint.ind_bit = be64_to_cpu(thinint.ind_bit);
+ thinint.summary_indicator =
+ be64_to_cpu(thinint.summary_indicator);
+ thinint.device_indicator =
+ be64_to_cpu(thinint.device_indicator);
dev->summary_indicator =
get_indicator(thinint.summary_indicator, sizeof(uint8_t));
@@ -654,8 +656,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
break;
}
ccw_dstream_read_buf(&sch->cds, &revinfo, 4);
- be16_to_cpus(&revinfo.revision);
- be16_to_cpus(&revinfo.length);
+ revinfo.revision = be16_to_cpu(revinfo.revision);
+ revinfo.length = be16_to_cpu(revinfo.length);
if (ccw.count < len + revinfo.length ||
(check_len && ccw.count > len + revinfo.length)) {
ret = -EINVAL;
--
2.19.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs
2018-12-10 12:04 [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs Peter Maydell
@ 2018-12-10 12:35 ` Philippe Mathieu-Daudé
2018-12-10 13:06 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
2018-12-10 15:22 ` [Qemu-devel] " Cornelia Huck
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-10 12:35 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: patches, Cornelia Huck, David Hildenbrand, Halil Pasic,
Christian Borntraeger, qemu-s390x, Richard Henderson
On 12/10/18 1:04 PM, Peter Maydell 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. Avoid the bug by not using the
> "modify in place" byte swapping functions.
>
> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci
> (with a couple of long lines manually wrapped).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> s390 also has some warnings in hw/s390x/css.c which are going to
> be harder to fix, relating to taking the address of the 'pmcw' and
> 'scsw' fields in the SCHUB struct...
>
> hw/s390x/virtio-ccw.c | 42 ++++++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 212b3d3dead..c2b78c8e9b1 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -287,18 +287,18 @@ static int virtio_ccw_handle_set_vq(SubchDev *sch, CCW1 ccw, bool check_len,
> }
> if (is_legacy) {
> ccw_dstream_read(&sch->cds, linfo);
> - be64_to_cpus(&linfo.queue);
> - be32_to_cpus(&linfo.align);
> - be16_to_cpus(&linfo.index);
> - be16_to_cpus(&linfo.num);
> + linfo.queue = be64_to_cpu(linfo.queue);
> + linfo.align = be32_to_cpu(linfo.align);
> + linfo.index = be16_to_cpu(linfo.index);
> + linfo.num = be16_to_cpu(linfo.num);
> ret = virtio_ccw_set_vqs(sch, NULL, &linfo);
> } else {
> ccw_dstream_read(&sch->cds, info);
> - be64_to_cpus(&info.desc);
> - be16_to_cpus(&info.index);
> - be16_to_cpus(&info.num);
> - be64_to_cpus(&info.avail);
> - be64_to_cpus(&info.used);
> + info.desc = be64_to_cpu(info.desc);
> + info.index = be16_to_cpu(info.index);
> + info.num = be16_to_cpu(info.num);
> + info.avail = be64_to_cpu(info.avail);
> + info.used = be64_to_cpu(info.used);
> ret = virtio_ccw_set_vqs(sch, &info, NULL);
> }
> sch->curr_status.scsw.count = 0;
> @@ -382,7 +382,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> features.features = 0;
> }
> ccw_dstream_rewind(&sch->cds);
> - cpu_to_le32s(&features.features);
> + features.features = cpu_to_le32(features.features);
> ccw_dstream_write(&sch->cds, features.features);
> sch->curr_status.scsw.count = ccw.count - sizeof(features);
> ret = 0;
> @@ -403,7 +403,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, features);
> - le32_to_cpus(&features.features);
> + features.features = le32_to_cpu(features.features);
> if (features.index == 0) {
> virtio_set_features(vdev,
> (vdev->guest_features & 0xffffffff00000000ULL) |
> @@ -546,7 +546,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, indicators);
> - be64_to_cpus(&indicators);
> + indicators = be64_to_cpu(indicators);
> dev->indicators = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -567,7 +567,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, indicators);
> - be64_to_cpus(&indicators);
> + indicators = be64_to_cpu(indicators);
> dev->indicators2 = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -588,14 +588,14 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, vq_config.index);
> - be16_to_cpus(&vq_config.index);
> + vq_config.index = be16_to_cpu(vq_config.index);
> if (vq_config.index >= VIRTIO_QUEUE_MAX) {
> ret = -EINVAL;
> break;
> }
> vq_config.num_max = virtio_queue_get_num(vdev,
> vq_config.index);
> - cpu_to_be16s(&vq_config.num_max);
> + vq_config.num_max = cpu_to_be16(vq_config.num_max);
> ccw_dstream_write(&sch->cds, vq_config.num_max);
> sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
> ret = 0;
> @@ -621,9 +621,11 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (ccw_dstream_read(&sch->cds, thinint)) {
> ret = -EFAULT;
> } else {
> - be64_to_cpus(&thinint.ind_bit);
> - be64_to_cpus(&thinint.summary_indicator);
> - be64_to_cpus(&thinint.device_indicator);
> + thinint.ind_bit = be64_to_cpu(thinint.ind_bit);
> + thinint.summary_indicator =
> + be64_to_cpu(thinint.summary_indicator);
> + thinint.device_indicator =
> + be64_to_cpu(thinint.device_indicator);
>
> dev->summary_indicator =
> get_indicator(thinint.summary_indicator, sizeof(uint8_t));
> @@ -654,8 +656,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> break;
> }
> ccw_dstream_read_buf(&sch->cds, &revinfo, 4);
> - be16_to_cpus(&revinfo.revision);
> - be16_to_cpus(&revinfo.length);
> + revinfo.revision = be16_to_cpu(revinfo.revision);
> + revinfo.length = be16_to_cpu(revinfo.length);
> if (ccw.count < len + revinfo.length ||
> (check_len && ccw.count > len + revinfo.length)) {
> ret = -EINVAL;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs
2018-12-10 12:04 [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs Peter Maydell
2018-12-10 12:35 ` Philippe Mathieu-Daudé
@ 2018-12-10 13:06 ` Halil Pasic
2018-12-10 13:39 ` Cornelia Huck
2018-12-10 15:22 ` [Qemu-devel] " Cornelia Huck
2 siblings, 1 reply; 5+ messages in thread
From: Halil Pasic @ 2018-12-10 13:06 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, Cornelia Huck, David Hildenbrand,
Christian Borntraeger, qemu-s390x, Richard Henderson
On Mon, 10 Dec 2018 12:04:36 +0000
Peter Maydell <peter.maydell@linaro.org> 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. Avoid the bug by not using the
> "modify in place" byte swapping functions.
>
> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci
> (with a couple of long lines manually wrapped).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
I wonder if packed was a good idea in the first place. @Connie: Do you
have an opinion on this?
Regards,
Halil
> ---
> s390 also has some warnings in hw/s390x/css.c which are going to
> be harder to fix, relating to taking the address of the 'pmcw' and
> 'scsw' fields in the SCHUB struct...
>
> hw/s390x/virtio-ccw.c | 42 ++++++++++++++++++++++--------------------
> 1 file changed, 22 insertions(+), 20 deletions(-)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 212b3d3dead..c2b78c8e9b1 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -287,18 +287,18 @@ static int virtio_ccw_handle_set_vq(SubchDev *sch, CCW1 ccw, bool check_len,
> }
> if (is_legacy) {
> ccw_dstream_read(&sch->cds, linfo);
> - be64_to_cpus(&linfo.queue);
> - be32_to_cpus(&linfo.align);
> - be16_to_cpus(&linfo.index);
> - be16_to_cpus(&linfo.num);
> + linfo.queue = be64_to_cpu(linfo.queue);
> + linfo.align = be32_to_cpu(linfo.align);
> + linfo.index = be16_to_cpu(linfo.index);
> + linfo.num = be16_to_cpu(linfo.num);
> ret = virtio_ccw_set_vqs(sch, NULL, &linfo);
> } else {
> ccw_dstream_read(&sch->cds, info);
> - be64_to_cpus(&info.desc);
> - be16_to_cpus(&info.index);
> - be16_to_cpus(&info.num);
> - be64_to_cpus(&info.avail);
> - be64_to_cpus(&info.used);
> + info.desc = be64_to_cpu(info.desc);
> + info.index = be16_to_cpu(info.index);
> + info.num = be16_to_cpu(info.num);
> + info.avail = be64_to_cpu(info.avail);
> + info.used = be64_to_cpu(info.used);
> ret = virtio_ccw_set_vqs(sch, &info, NULL);
> }
> sch->curr_status.scsw.count = 0;
> @@ -382,7 +382,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> features.features = 0;
> }
> ccw_dstream_rewind(&sch->cds);
> - cpu_to_le32s(&features.features);
> + features.features = cpu_to_le32(features.features);
> ccw_dstream_write(&sch->cds, features.features);
> sch->curr_status.scsw.count = ccw.count - sizeof(features);
> ret = 0;
> @@ -403,7 +403,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, features);
> - le32_to_cpus(&features.features);
> + features.features = le32_to_cpu(features.features);
> if (features.index == 0) {
> virtio_set_features(vdev,
> (vdev->guest_features & 0xffffffff00000000ULL) |
> @@ -546,7 +546,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, indicators);
> - be64_to_cpus(&indicators);
> + indicators = be64_to_cpu(indicators);
> dev->indicators = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -567,7 +567,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, indicators);
> - be64_to_cpus(&indicators);
> + indicators = be64_to_cpu(indicators);
> dev->indicators2 = get_indicator(indicators, sizeof(uint64_t));
> sch->curr_status.scsw.count = ccw.count - sizeof(indicators);
> ret = 0;
> @@ -588,14 +588,14 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> ret = -EFAULT;
> } else {
> ccw_dstream_read(&sch->cds, vq_config.index);
> - be16_to_cpus(&vq_config.index);
> + vq_config.index = be16_to_cpu(vq_config.index);
> if (vq_config.index >= VIRTIO_QUEUE_MAX) {
> ret = -EINVAL;
> break;
> }
> vq_config.num_max = virtio_queue_get_num(vdev,
> vq_config.index);
> - cpu_to_be16s(&vq_config.num_max);
> + vq_config.num_max = cpu_to_be16(vq_config.num_max);
> ccw_dstream_write(&sch->cds, vq_config.num_max);
> sch->curr_status.scsw.count = ccw.count - sizeof(vq_config);
> ret = 0;
> @@ -621,9 +621,11 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> if (ccw_dstream_read(&sch->cds, thinint)) {
> ret = -EFAULT;
> } else {
> - be64_to_cpus(&thinint.ind_bit);
> - be64_to_cpus(&thinint.summary_indicator);
> - be64_to_cpus(&thinint.device_indicator);
> + thinint.ind_bit = be64_to_cpu(thinint.ind_bit);
> + thinint.summary_indicator =
> + be64_to_cpu(thinint.summary_indicator);
> + thinint.device_indicator =
> + be64_to_cpu(thinint.device_indicator);
>
> dev->summary_indicator =
> get_indicator(thinint.summary_indicator, sizeof(uint8_t));
> @@ -654,8 +656,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
> break;
> }
> ccw_dstream_read_buf(&sch->cds, &revinfo, 4);
> - be16_to_cpus(&revinfo.revision);
> - be16_to_cpus(&revinfo.length);
> + revinfo.revision = be16_to_cpu(revinfo.revision);
> + revinfo.length = be16_to_cpu(revinfo.length);
> if (ccw.count < len + revinfo.length ||
> (check_len && ccw.count > len + revinfo.length)) {
> ret = -EINVAL;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [qemu-s390x] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs
2018-12-10 13:06 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
@ 2018-12-10 13:39 ` Cornelia Huck
0 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2018-12-10 13:39 UTC (permalink / raw)
To: Halil Pasic
Cc: Peter Maydell, qemu-devel, patches, David Hildenbrand,
Christian Borntraeger, qemu-s390x, Richard Henderson
On Mon, 10 Dec 2018 14:06:42 +0100
Halil Pasic <pasic@linux.ibm.com> wrote:
> On Mon, 10 Dec 2018 12:04:36 +0000
> Peter Maydell <peter.maydell@linaro.org> 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. Avoid the bug by not using the
> > "modify in place" byte swapping functions.
> >
> > Patch produced with scripts/coccinelle/inplace-byteswaps.cocci
> > (with a couple of long lines manually wrapped).
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>
> Reviewed-by: Halil Pasic <pasic@linux.ibm.com>
>
> I wonder if packed was a good idea in the first place. @Connie: Do you
> have an opinion on this?
Some of the structures probably don't need the packed attribute; but
there are two structures that have a trailing 8 bit value. The problem
is that the structures are a a guest<->host interface as defined in the
virtio standard, so we can't e.g. add padding to the structures with
the isc at the end.
If I had to re-define the interface, I'd probably add tail padding; but
I was not aware of the problems the packing might cause when I defined
them, and we can't get rid of them without also getting rid of support
for older virtio versions.
Simply moving away from the modify in place byteswaps is the easiest
solution.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs
2018-12-10 12:04 [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs Peter Maydell
2018-12-10 12:35 ` Philippe Mathieu-Daudé
2018-12-10 13:06 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
@ 2018-12-10 15:22 ` Cornelia Huck
2 siblings, 0 replies; 5+ messages in thread
From: Cornelia Huck @ 2018-12-10 15:22 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, patches, qemu-s390x, Halil Pasic,
Christian Borntraeger, Richard Henderson, David Hildenbrand
On Mon, 10 Dec 2018 12:04:36 +0000
Peter Maydell <peter.maydell@linaro.org> 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. Avoid the bug by not using the
> "modify in place" byte swapping functions.
>
> Patch produced with scripts/coccinelle/inplace-byteswaps.cocci
> (with a couple of long lines manually wrapped).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Thanks, applied.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-12-10 15:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-10 12:04 [Qemu-devel] [PATCH] hw/s390x/virtio-ccw.c: Don't take address of fields in packed structs Peter Maydell
2018-12-10 12:35 ` Philippe Mathieu-Daudé
2018-12-10 13:06 ` [Qemu-devel] [qemu-s390x] " Halil Pasic
2018-12-10 13:39 ` Cornelia Huck
2018-12-10 15:22 ` [Qemu-devel] " Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).