netdev - Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time

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] [day] [month] [year] [list]

Message-ID: <CA+sq2Cff3tNY1=wS30G4gW_9dqR1rOOQrW1m5d5Mw6+g68hKfA@mail.gmail.com>
Date:   Mon, 12 Nov 2018 16:01:11 +0530
From:   Sunil Kovvuri <sunil.kovvuri@...il.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>, linux-soc@...r.kernel.org,
        Sunil Goutham <sgoutham@...vell.com>
Subject: Re: [PATCH 08/20] octeontx2-af: Alloc and config NPC MCAM entry at a time

On Sat, Nov 10, 2018 at 2:36 AM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Fri, Nov 9, 2018 at 6:13 PM Sunil Kovvuri <sunil.kovvuri@...il.com> wrote:
> > On Fri, Nov 9, 2018 at 4:32 PM Arnd Bergmann <arnd@...db.de> wrote:
> > > On Fri, Nov 9, 2018 at 5:21 AM Sunil Kovvuri <sunil.kovvuri@...il.com> wrote:
>
> > >
> > > Since b is aligned to four bytes, you get padding between a and b.
> > > On top of that, you also get padding after c to make the size of
> > > structure itself be a multiple of its alignment. For interfaces, we
> > > should avoid both kinds of padding. This can be done by marking
> > > members as __packed (usually I don't recommend that), by
> > > changing the size of members, or by adding explicit 'reserved'
> > > fields in place of the padding.
> > >
> > > > > I also noticed a similar problem in struct mbox_msghdr. Maybe
> > > > > use the 'pahole' tool to check for this kind of padding in the
> > > > > API structures.
> >
> > Got your point now and agree that padding has to be avoided.
> > But this is a big change and above pointed structure is not
> > the only one as this applies to all structures in the file.
> >
> > Would it be okay if I submit a separate patch after this series
> > addressing all structures ?
>
> It depends on how you want to address it. If you want to
> change the structure layout, then I think it would be better
> integrated into the series as that is an incompatible interface
> change. If you just want to add reserved members to make
> the padding explicit, that could be a follow-up.
>
>         Arnd

Upon further thought i think padding is needed here but adding reserved fields
is also not enough. Probably i will have to align all struct elements to 64bit
with  __attribute__ ((aligned(8))) to avoid unaligned access faults when device
is attached to guest. VFIO maps BARs as device memory, so irrespective of
whatever mapping i do in driver, reads/writes to the mailbox region might result
in unaligned access fault.

Checking if there are other ways of solving it.

Thanks,
Sunil.