netdev - Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)

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, 14 Oct 2016 09:25:16 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Andy Lutomirski <luto@...capital.net>
Cc:     Stephen Rothwell <sfr@...b.auug.org.au>,
        "linux-next@...r.kernel.org" <linux-next@...r.kernel.org>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Linux Wireless List <linux-wireless@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
Subject: Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)

On Thu, 2016-10-13 at 14:49 -0700, Andy Lutomirski wrote:
> 
> It's failing before that.  With CONFIG_VMAP_STACK=y, the stack may
> not be physically contiguous and can't be used for DMA, so putting it
> in a scatterlist is bogus in general, and the crypto code mostly
> wants a scatterlist.

I see, so all this stuff is getting inlined, and we crash in
sg_set_buf() because it does sg_set_page() and that obviously needs to
do virt_to_page(), which is invalid on this address now.
With CONFIG_DEBUG_SG we'd have hit the BUG_ON there instead.

It does indeed look like AEAD doesn't have any non-SG API.

So ultimately, the bug already goes back to Ard's commit 7ec7c4a9a686
("mac80211: port CCMP to cryptoapi's CCM driver") since that already
potentially used stack space for DMA.

Since we don't have any space in the SKB or anywhere else at this point
(other than the stack that we can't use), I see two ways out of this:

   1. revert that patch (doing so would need some major adjustments now,
      since it's pretty old and a number of new things were added in the
      meantime)
   2. allocate a per-CPU buffer for all the things that we put on the
      stack and use in SG lists, those are:
       * CCM/GCM: AAD (32B), B_0/J_0 (16B)
       * GMAC: AAD (20B), zero (16B)
       * (not sure why CMAC isn't using this API, but it would be like
      GMAC)

Thoughts?

johannes