Thread (31 messages) 31 messages, 11 authors, 2021-09-17

Re: ipv4/tcp.c:4234:1: error: the frame size of 1152 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]

From: Johannes Berg <johannes@sipsolutions.net>
Date: 2021-09-08 07:09:25
Also in: linux-arm-kernel, lkml

On Tue, 2021-09-07 at 16:14 -0700, Linus Torvalds wrote:
The mac802.11 one seems to be due to 'struct ieee802_11_elems' being
big, and allocated on the stack. I think it's probably made worse
there with inlining, ie

 - ieee80211_sta_rx_queued_mgmt() has one copy

 - ieee80211_rx_mgmt_beacon() is possibly inlined, and has its own copy

but even if it isn't due to that kind of duplication due to inlining,
that code is dangerous. Exactly because it has two nested stack frames
with that big structure, and they are active at the same time in the
callchain whether inlined or not.

And it's *pointlessly* dangerous, because the 'struct ieee802_11_elems
elems' in ieee80211_sta_rx_queued_mgmt() is only used for the
IEEE80211_STYPE_ACTION case, so it is entirely disjoint from the
IEEE80211_STYPE_BEACON case, and those stack allocations simply should
not nest like that in the first place.

Making the IEEE80211_STYPE_ACTION case be its own function - like the
other cases - and moving the struct there should fix it. Possibly a
"noinline" or two necessary to make sure that the compiler doesn't
then undo the "these two cases are disjoint" thing.
Yeah, I'm aware, and I agree. We've been looking at it every now and
then. This got made worse by us actually adding a fair amount of
pointers to the struct recently (in this merge window).

Ultimately, every new spec addition ends up needing to add something
there, so I think ultimately we'll probably want to either dynamically
allocate it somewhere (perhaps in a data structure used here already),
or possibly not have this at all and just find a way to return only the
bits that are interesting. Even parsing a ~1k frame (typical, max ~2k) a
handful of times is probably not even worse than having this large a
structure that gets filled data that's probably useless in many cases (I
think the different cases all just need a subset). But not sure, I'll
take a look.

johannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help