Thread (33 messages) 33 messages, 8 authors, 2017-08-04

Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2017-06-16 17:29:49
Also in: lkml

On Fri, Jun 16, 2017 at 8:58 AM, Samuel Thibault
[off-list ref] wrote:
Arnd Bergmann, on ven. 16 juin 2017 17:41:47 +0200, wrote:
quoted
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.
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help