[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/12] slirp: remove QEMU_PACKED from structures
From: |
Samuel Thibault |
Subject: |
Re: [Qemu-devel] [PATCH 01/12] slirp: remove QEMU_PACKED from structures with don't require it |
Date: |
Tue, 9 Jan 2018 21:54:54 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
Hello,
Philippe Mathieu-Daudé, on lun. 08 janv. 2018 14:28:53 -0300, wrote:
> struct mbuf_ptr {
> struct mbuf *mptr;
> uint32_t dummy;
> -} QEMU_PACKED;
> +};
> #else
> struct mbuf_ptr {
> struct mbuf *mptr;
> -} QEMU_PACKED;
> +};
> @@ -199,7 +199,7 @@ struct ipovly {
> uint16_t ih_len; /* protocol length */
> struct in_addr ih_src; /* source internet address */
> struct in_addr ih_dst; /* destination internet address */
> -} QEMU_PACKED;
> +};
Did you actually try to change these structures? The presence of the
"dummy" field should have hinted that it's not a trivial structure :)
mbuf_ptr is used in struct ipovly which is to have the same size as the
ipv4 header. One would have to untangle that before removing the packed
attribute.
> @@ -215,7 +215,7 @@ struct ipq {
> uint8_t ipq_p; /* protocol of this fragment */
> uint16_t ipq_id; /* sequence id for reassembly */
> struct in_addr ipq_src,ipq_dst;
> -} QEMU_PACKED;
> +};
This one seems safe to me. I'd still rather see an actual test with
added holes in the structure to be sure :)
> @@ -225,7 +225,7 @@ struct ipq {
> struct ipasfrag {
> struct qlink ipf_link;
> struct ip ipf_ip;
> -} QEMU_PACKED;
> +};
Please see iptofrag and fragtoip in ip_input.c, they assume that ipf_ip
immediately follows ipf_link. I guess using offsetof there should avoid
the issue. Note however that slirp assumes that in a 32bit-aligned
ethernet frame it has enough room before the IP header to stuff the
ipf_link things. We could add a build-time check that offsetof(ipf_ip)
<= 2 + ETH_HLEN
> @@ -136,7 +136,7 @@ bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
> struct ndpentry {
> unsigned char eth_addr[ETH_ALEN]; /* sender hardware address */
> struct in6_addr ip_addr; /* sender IP address */
> -} QEMU_PACKED;
> +};
This one should be safe.
Samuel
- [Qemu-devel] [PATCH 00/12] add HOST_SUPPORTS_UNALIGNED_ACCESS, reduce slirp warnings, Philippe Mathieu-Daudé, 2018/01/08
- [Qemu-devel] [PATCH 01/12] slirp: remove QEMU_PACKED from structures with don't require it, Philippe Mathieu-Daudé, 2018/01/08
- [Qemu-devel] [PATCH 02/12] slirp: struct icmp/ethhdr ARE packed, Philippe Mathieu-Daudé, 2018/01/08
- [Qemu-devel] [PATCH 03/12] slirp: avoid IN6_IS_ADDR_UNSPECIFIED(), rather use in6_zero(), Philippe Mathieu-Daudé, 2018/01/08
- [Qemu-devel] [PATCH 04/12] slirp: add in6_multicast() and use it instead of IN6_IS_ADDR_MULTICAST(), Philippe Mathieu-Daudé, 2018/01/08
- [Qemu-devel] [PATCH 05/12] slirp: poison IN6_*_ADDR_*() macros to avoid them, Philippe Mathieu-Daudé, 2018/01/08