netdev - Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties

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]

Message-ID: <20170927113241.GB4630@lahna.fi.intel.com>
Date:   Wed, 27 Sep 2017 14:32:41 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     David Miller <davem@...emloft.net>
Cc:     gregkh@...uxfoundation.org, andreas.noever@...il.com,
        michael.jamet@...el.com, yehezkel.bernat@...el.com,
        amir.jer.levy@...el.com, Mario.Limonciello@...l.com,
        lukas@...ner.de, andriy.shevchenko@...ux.intel.com, andrew@...n.ch,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2 02/16] thunderbolt: Add support for XDomain properties

On Tue, Sep 26, 2017 at 09:33:54PM -0700, David Miller wrote:
> From: Mika Westerberg <mika.westerberg@...ux.intel.com>
> Date: Mon, 25 Sep 2017 14:07:24 +0300
> 
> > +struct tb_property_entry {
> > +	u32 key_hi;
> > +	u32 key_lo;
> > +	u16 length;
> > +	u8 reserved;
> > +	u8 type;
> > +	u32 value;
> > +} __packed;
> > +
> > +struct tb_property_rootdir_entry {
> > +	u32 magic;
> > +	u32 length;
> > +	struct tb_property_entry entries[];
> > +} __packed;
> > +
> > +struct tb_property_dir_entry {
> > +	u32 uuid[4];
> > +	struct tb_property_entry entries[];
> > +} __packed;
> 
> There is no apparent need for __packed here, and __packed should be
> avoided unless absolutely necessary as it pessimizes the code
> significantly on some architectures.
> 
> Please remove __packed from these datastructures unless you can
> prove it is absolutely needed and, in such case, please document
> in a comment why that requirement exists.  Because from the layout
> of these types, everything will be packed in just fine without
> __packed.

I will thanks.

Just for my education, is there some rule which tells when __packed is
to be used? For example the above structures are all 32-bit aligned but
how about something like:

struct foo {
	u32 value1;
	u8 value2;
};

If the on-wire format requires such structures I assume __packed
is needed here?

Thanks!