netdev - Re: pull-request: mac80211-next 2019-07-31

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]

Message-ID: <2f64367daad256b1f1999797786763fa8091faa1.camel@sipsolutions.net>
Date:   Mon, 28 Oct 2019 10:51:51 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     David Miller <davem@...emloft.net>,
        Networking <netdev@...r.kernel.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        John Crispin <john@...ozen.org>
Subject: Re: pull-request: mac80211-next 2019-07-31

Hi Arnd,

> It looks like one of the last additions pushed the stack usage over
> the 1024 byte limit
> for 32-bit architectures:
> 
> net/mac80211/mlme.c:4063:6: error: stack frame size of 1032 bytes in
> function 'ieee80211_sta_rx_queued_mgmt' [-Werror,-Wframe-larger-than=]
> 
> struct ieee802_11_elems is fairly large, and just grew another two pointers.
> When ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success()
> are inlined into ieee80211_sta_rx_queued_mgmt(), there are three copies
> of this structure, which is slightly too much.

Hmm. I guess that means the compiler isn't smart enough to make the
copies from the inlined sub-functions alias each other? I mean, if I
have

fn1(...) { struct ... elems1; ... }
fn2(...) { struct ... elems2; ... }

fn(...)
{
  fn1();
  fn2();
}

then it could reasonably use the same stack memory for elems1 and
elems2, at least theoretically, but you're saying it doesn't do that I
guess?

It could even do that for different BBs, in theory ...

If it does, I'd have suggested to move the code from the outer function
inside the "case IEEE80211_STYPE_ACTION:" block into a new sub-function, 
but that won't work then.

I don't think dynamic allocation would be nice - but we could manually
do this by passing the elems pointer into the
ieee80211_rx_mgmt_assoc_resp() and ieee80211_assoc_success() functions.

Why do you say 32-bit btw, it should be *bigger* on 64-bit, but I didn't
see this ... hmm.

johannes