Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752310AbdFPR3t (ORCPT ); Fri, 16 Jun 2017 13:29:49 -0400 Received: from mail-ua0-f195.google.com ([209.85.217.195]:36107 "EHLO mail-ua0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750780AbdFPR3r (ORCPT ); Fri, 16 Jun 2017 13:29:47 -0400 MIME-Version: 1.0 In-Reply-To: <20170616155859.gn2einuermlncaku@var.youpi.perso.aquilenet.fr> References: <20170614211556.2062728-1-arnd@arndb.de> <20170614211556.2062728-4-arnd@arndb.de> <20170615045221.GA26687@kroah.com> <20170615045347.GA26913@kroah.com> <20170616130215.GC31057@kroah.com> <20170616155859.gn2einuermlncaku@var.youpi.perso.aquilenet.fr> From: Dmitry Torokhov Date: Fri, 16 Jun 2017 10:29:45 -0700 Message-ID: Subject: Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN To: Samuel Thibault , Arnd Bergmann , Greg Kroah-Hartman , Andrew Morton , kasan-dev , Dmitry Vyukov , Alexander Potapenko , Andrey Ryabinin , Networking , Linux Kernel Mailing List , Arend van Spriel , Jiri Slaby Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1720 Lines: 47 On Fri, Jun 16, 2017 at 8:58 AM, Samuel Thibault wrote: > Arnd Bergmann, on ven. 16 juin 2017 17:41:47 +0200, wrote: >> The problem are the 'ch' and 'flag' variables that are passed into >> tty_insert_flip_char by value, and from there into >> tty_insert_flip_string_flags by reference. In this case, kasan tries >> to detect whether tty_insert_flip_string_flags() does any out-of-bounds >> access on the pointers and adds 64 bytes redzone around each of >> the two variables. > > Ouch. > >> gcc-6.3.1 happens to inline 16 calls of tty_insert_flip_char() into I wonder if we should stop marking tty_insert_flip_char() as inline. >> kbd_keycode(), so the stack size grows from 168 bytes to >> 168+(16*2*64) = 2216 bytes. There are 10 calls to put_queue() >> in to_utf8(), 12 in emulate_raw() and another 4 in kbd_keycode() >> itself. > > That's why I agreed for put_queue :) > > I'm however afraid we'd have to mark a lot of static functions that way, > depending on the aggressivity of gcc... I'd indeed really argue that gcc > should consider stack usage when inlining. > > static int f(int foo) { > char c[256]; > g(c, foo); > } > > is really not something that I'd want to see the compiler to inline. Why would not we want it be inlined? What we do not want us several calls having _separate_ instances of 'c' generated on the stack, all inlined calls should share 'c'. And of course if we have f1, f2, and f3 with c1, c2, and c3, GCC should not blow up the stack inlining and allocating stack for all 3 of them beforehand. But this all seems to me issue that should be solved in toolchain, not trying to play whack-a-mole with kernel sources. Thanks. -- Dmitry