netdev - Re: [PATCH v2 2/2] r8169: Avoid pointer aliasing

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]

Date:   Wed, 6 Feb 2019 08:25:30 +0100
From:   Michal Kubecek <mkubecek@...e.cz>
To:     netdev@...r.kernel.org
Cc:     Joe Perches <joe@...ches.com>, David Miller <davem@...emloft.net>,
        thierry.reding@...il.com, hkallweit1@...il.com, andrew@...n.ch,
        nic_swsd@...ltek.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] r8169: Avoid pointer aliasing

On Tue, Feb 05, 2019 at 11:19:04AM -0800, Joe Perches wrote:
> On Tue, 2019-02-05 at 11:14 -0800, David Miller wrote:
> > From: Joe Perches <joe@...ches.com>
> > Date: Tue, 05 Feb 2019 10:42:54 -0800
> > 
> > > On Mon, 2019-02-04 at 19:20 -0800, David Miller wrote:
> > >> From: Thierry Reding <thierry.reding@...il.com>
> > >> Date: Mon,  4 Feb 2019 17:42:13 +0100
> > >> 
> > >> > @@ -7316,7 +7325,7 @@ static int rtl_get_ether_clk(struct rtl8169_private *tp)
> > >> >  static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >> >  {
> > >> >       const struct rtl_cfg_info *cfg = rtl_cfg_infos + ent->driver_data;
> > >> > -     u8 mac_addr[ETH_ALEN] __aligned(4) = {};
> > >> > +     u8 mac_addr[ETH_ALEN] = {};
> > >> >       struct rtl8169_private *tp;
> > >> 
> > >> I agree with Heiner, you have to provide at least 2 byte alignment for this
> > >> buffer due to the reasons he stated.
> > > 
> > > It's declared after a pointer so it is already is 2 byte aligned.
> > > 
> > > A lot of drivers wouldn't work otherwise.
> > 
> > That's assuming a lot about what the compiler will do when nit allocates
> > local variables to the stack.
> 
> It's also assuming what a compiler will do when
> it defines a struct.

This is not a structure member, it's a local variable. I'm not aware of
any C norm requirement for ordering of local variables on the stack.
After all, some of them might not be on the stack at all and use only
registers or be optimized out completely.

Michal Kubecek