linux-kernel - Re: [PATCH linux-next] net: batman-adv: use "__packed __aligned(2)" for each structure instead of "_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 18 Jan 2014 14:03:26 +0100
From:	Antonio Quartulli <antonio@...hcoding.com>
To:	Chen Gang <gang.chen.5i5j@...il.com>,
	David Miller <davem@...emloft.net>
CC:	mareklindner@...mailbox.ch, sw@...onwunderlich.de,
	b.a.t.m.a.n@...ts.open-mesh.org, netdev <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-metag@...r.kernel.org, James Hogan <james.hogan@...tec.com>
Subject: Re: [PATCH linux-next] net: batman-adv: use "__packed __aligned(2)"
 for each structure instead of "__packed(2)" region

On 18/01/14 12:31, Chen Gang wrote:
> Unfortunately, not all compilers assumes the structures within a pack
> region also need be packed (e.g. metag), so need add a pack explicitly
> to satisfy all compilers.
> 
> The related error (under metag with allmodconfig):
> 
>     MODPOST 2952 modules
>   ERROR: "__compiletime_assert_431" [net/batman-adv/batman-adv.ko] undefined!
>   ERROR: "__compiletime_assert_432" [net/batman-adv/batman-adv.ko] undefined!
>   ERROR: "__compiletime_assert_429" [net/batman-adv/batman-adv.ko] undefined!
>   ERROR: "__compiletime_assert_428" [net/batman-adv/batman-adv.ko] undefined!
>   ERROR: "__compiletime_assert_423" [net/batman-adv/batman-adv.ko] undefined!
> 
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@...il.com>

David, what do you think about this change?


Can "__packed __aligned(2)" generate a different structure padding than
"#pragma pack(2)" ?

I am not really sure about the difference between the two. But if we
have the possibility that the padding may change then this patch should
go into net, otherwise we will have a protocol compatibility problem
between 3.13 and 3.14.


Cheers,

> ---
>  net/batman-adv/packet.h | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
> index 0a381d1..9206b48 100644
> --- a/net/batman-adv/packet.h
> +++ b/net/batman-adv/packet.h
> @@ -154,7 +154,6 @@ enum batadv_tvlv_type {
>  	BATADV_TVLV_ROAM	= 0x05,
>  };
>  
> -#pragma pack(2)
>  /* the destination hardware field in the ARP frame is used to
>   * transport the claim type and the group id
>   */
> @@ -162,8 +161,7 @@ struct batadv_bla_claim_dst {
>  	uint8_t magic[3];	/* FF:43:05 */
>  	uint8_t type;		/* bla_claimframe */
>  	__be16 group;		/* group id */
> -};
> -#pragma pack()
> +} __packed __aligned(2);
>  
>  /**
>   * struct batadv_ogm_packet - ogm (routing protocol) packet
> @@ -281,7 +279,6 @@ struct batadv_icmp_packet_rr {
>   * misalignment of the payload after the ethernet header. It may also lead to
>   * leakage of information when the padding it not initialized before sending.
>   */
> -#pragma pack(2)
>  
>  /**
>   * struct batadv_unicast_packet - unicast packet for network payload
> @@ -300,7 +297,7 @@ struct batadv_unicast_packet {
>  	/* "4 bytes boundary + 2 bytes" long to make the payload after the
>  	 * following ethernet header again 4 bytes boundary aligned
>  	 */
> -};
> +}  __packed __aligned(2);
>  
>  /**
>   * struct batadv_unicast_4addr_packet - extended unicast packet
> @@ -316,7 +313,7 @@ struct batadv_unicast_4addr_packet {
>  	/* "4 bytes boundary + 2 bytes" long to make the payload after the
>  	 * following ethernet header again 4 bytes boundary aligned
>  	 */
> -};
> +}  __packed __aligned(2);
>  
>  /**
>   * struct batadv_frag_packet - fragmented packet
> @@ -347,7 +344,7 @@ struct batadv_frag_packet {
>  	uint8_t orig[ETH_ALEN];
>  	__be16  seqno;
>  	__be16  total_size;
> -};
> +}  __packed __aligned(2);
>  
>  /**
>   * struct batadv_bcast_packet - broadcast packet for network payload
> @@ -368,7 +365,7 @@ struct batadv_bcast_packet {
>  	/* "4 bytes boundary + 2 bytes" long to make the payload after the
>  	 * following ethernet header again 4 bytes boundary aligned
>  	 */
> -};
> +}  __packed __aligned(2);
>  
>  /**
>   * struct batadv_coded_packet - network coded packet
> @@ -404,9 +401,8 @@ struct batadv_coded_packet {
>  	uint8_t  second_orig_dest[ETH_ALEN];
>  	__be32   second_crc;
>  	__be16   coded_len;
> -};
> +}  __packed __aligned(2);
>  
> -#pragma pack()
>  
>  /**
>   * struct batadv_unicast_tvlv - generic unicast packet with tvlv payload
> 


-- 
Antonio Quartulli


Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)