linux-kernel - Re: [PATCH anybus v3 4/6] bus: support HMS Anybus-S bus

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:   Fri, 9 Nov 2018 11:25:09 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Sven Van Asbroeck <svendev@...x.com>, robh+dt@...nel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Lee Jones <lee.jones@...aro.org>, mark.rutland@....com,
        Andreas Färber <afaerber@...e.de>,
        treding@...dia.com, David Lechner <david@...hnology.com>,
        noralf@...nnes.org, johan@...nel.org,
        Michal Simek <monstr@...str.eu>, michal.vokac@...ft.com,
        gregkh@...uxfoundation.org, john.garry@...wei.com,
        geert+renesas@...der.be, robin.murphy@....com,
        paul.gortmaker@...driver.com,
        sebastien.bourdelin@...oirfairelinux.com, icenowy@...c.io,
        Stuart Yoder <stuyoder@...il.com>, maxime.ripard@...tlin.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCH anybus v3 4/6] bus: support HMS Anybus-S bus

On Thu, Nov 8, 2018 at 9:07 AM Arnd Bergmann <arnd@...db.de> wrote:
>
> > +struct anybus_mbox_hdr {
> > +       u16 id;
> > +       u16 info;
> > +       u16 cmd_num;
> > +       u16 data_size;
> > +       u16 frame_count;
> > +       u16 frame_num;
> > +       u16 offset_high;
> > +       u16 offset_low;
> > +       u16 extended[8];
> > +} __packed;
>
> I don't think you want this to be __packed, it won't change the layout
> but instead force byte accesses on architectures that don't have
> fast unaligned load/store instructions.
>
> Instead of the __packed, it's almost always better to ensure that
> the structure itself is aligned to a 16-bit address.
>

A general question about __packed.

My current understanding is this:
(please tell me if it's incorrect or incomplete)

+ without __packed, the compiler is free to pad the struct in whatever
way it feels is best.
+ with __packed, the fields have to be laid out EXACTLY as specified.

Is it possible that someone will compile this on an architecture that 'likes'
16-bit ints to be 32-bit aligned? If such an architecture does not currently
exist, could it appear in the future?

If the compiler changes the padding in the struct, the driver will
break, as the struct layout is tightly defined by the anybus spec.

Would it be better to stay safe, and keep __packed in case of such
tightly defined structures?