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
| ||
|
Date: Sat, 7 Sep 2019 20:41:22 +0200 From: Arnd Bergmann <arnd@...db.de> To: Pablo Neira Ayuso <pablo@...filter.org> Cc: Jozsef Kadlecsik <kadlec@...filter.org>, Florian Westphal <fw@...len.de>, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <jakub.kicinski@...ronome.com>, wenxu <wenxu@...oud.cn>, netfilter-devel <netfilter-devel@...r.kernel.org>, coreteam@...filter.org, Networking <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net-next] netfilter: nf_tables: avoid excessive stack usage On Sat, Sep 7, 2019 at 8:07 PM Pablo Neira Ayuso <pablo@...filter.org> wrote: > > Hi Arnd, > > On Fri, Sep 06, 2019 at 05:12:30PM +0200, Arnd Bergmann wrote: > > The nft_offload_ctx structure is much too large to put on the > > stack: > > > > net/netfilter/nf_tables_offload.c:31:23: error: stack frame size of 1200 bytes in function 'nft_flow_rule_create' [-Werror,-Wframe-larger-than=] > > > > Use dynamic allocation here, as we do elsewhere in the same > > function. > > > > Fixes: c9626a2cbdb2 ("netfilter: nf_tables: add hardware offload support") > > Signed-off-by: Arnd Bergmann <arnd@...db.de> > > --- > > Since we only really care about two members of the structure, an > > alternative would be a larger rewrite, but that is probably too > > late for v5.4. > > Thanks for this patch. > > I'm attaching a patch to reduce this structure size a bit. Do you > think this alternative patch is ok until this alternative rewrite > happens? I haven't tried it yet, but it looks like that would save 8 of the 48 bytes in each for each of the 24 registers (12 bytes on m68k or i386, which only use 4 byte alignment for nft_data), so this wouldn't make too much difference. > Anyway I agree we should to get this structure away from the > stack, even after this is still large, so your patch (or a variant of > it) will be useful sooner than later I think. What I was thinking for a possible smaller fix would be to not pass the ctx into the expr->ops->offload callback but only pass the 'dep' member. Since I've never seen this code before, I have no idea if that would be an improvement in the end. Arnd
Powered by blists - more mailing lists