netdev - Re: [PATCH] IB/IPoIB: Check the headroom size

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:   Tue, 25 Apr 2017 11:40:15 -0400
From:   Doug Ledford <dledford@...hat.com>
To:     Or Gerlitz <gerlitz.or@...il.com>,
        Erez Shitrit <erezsh@....mellanox.co.il>,
        Paolo Abeni <pabeni@...hat.com>
Cc:     Honggang LI <honli@...hat.com>, Erez Shitrit <erezsh@...lanox.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        Linux Netdev List <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH] IB/IPoIB: Check the headroom size

On Tue, 2017-04-25 at 17:39 +0300, Or Gerlitz wrote:
> On Tue, Apr 25, 2017 at 2:43 PM, Erez Shitrit <erezsh@....mellanox.co
> .il> wrote:
> > 
> > On Tue, Apr 25, 2017 at 2:14 PM, Or Gerlitz <gerlitz.or@...il.com>
> > wrote:
> 
> > 
> > > 
> > > thanks for the info. Is this bug there since ipoib/bonding day
> > > one (and hence my bug...)
> > > or was indeed introduced later? if later, can you explain how
> > > fc791b633515 introduced that or you only know it by bisection?
> 
> > 
> > commit "fc791b633515" changes the size of the dev_hardlen to be 24
> > and
> > required 24 extra bytes in the skb, before it was only 4, if skb is
> > aligned to eth "mode" it already has 14 bytes for hard-header.
> > So only after that commit we have the issue.
> 
> If got you right, Paolo's commit introduced a regression, so we (I
> guess you and
> Paolo) need to either solve it or we (community) should consider a
> revert, please suggest.

It's a little more complex than that.  Paolo's commit *re-introduced* a
regression.  If you recall, long ago the IPoIB layer stuck the dgid
into the skb, then pulled it out later.  However, we didn't actually
declare things properly back then, but it worked anyway.  Then we had
the commit you authored to start using the skb->cb to store the dgid,
and our usage of hard header dropped to only 4 bytes.  Paolo's commit
went back to the old way of doing things, but also did the proper
accounting and setup to tell the netstack what we were doing (which the
initial implementation never did IIRC).  So, this issue should be
reproducible either after Paolo's commit or with any kernel prior to
your commit to use the skb->cb area to store the DGID, but it probably
requires the specific series of actions in this bug to trigger it.  A
normal, clean shutdown of the interface doesn't demonstrate the issue.

> The bug is now in stable and distro kernels, so please act.

-- 
Doug Ledford <dledford@...hat.com>
    GPG KeyID: B826A3330E572FDD

Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD