Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN
From: Samuel Thibault <samuel.thibault@ens-lyon.org>
Date: 2017-06-14 21:28:39
Also in:
lkml
Hello, Arnd Bergmann, on mer. 14 juin 2017 23:15:38 +0200, wrote:
As reported by kernelci, some functions in the VT code use significant amounts of kernel stack when local variables get inlined into the caller multiple times: drivers/tty/vt/keyboard.c: In function 'kbd_keycode': drivers/tty/vt/keyboard.c:1452:1: error: the frame size of 2240 bytes is larger than 2048 bytes [-Werror=frame-larger-than=] Annotating those functions as noinline_if_stackbloat prevents the inlining and reduces the overall stack usage in this driver.
quoted hunk ↗ jump to hunk
--- a/drivers/tty/vt/keyboard.c +++ b/drivers/tty/vt/keyboard.c@@ -301,13 +301,13 @@ int kbd_rate(struct kbd_repeat *rpt) /* * Helper Functions. */ -static void put_queue(struct vc_data *vc, int ch) +static noinline_if_stackbloat void put_queue(struct vc_data *vc, int ch) { tty_insert_flip_char(&vc->port, ch, 0); tty_schedule_flip(&vc->port); }
I'm surprised that this be able generate so much stack use: the tty_insert_flip_char inline only uses a pointer and an int. And I'm surprised that multiple inlines can accumulate stack usage. I however agree that it's a bad idea to inline it in functions where it's called so many times (and we're talking about the keyboard anyway).
-static void puts_queue(struct vc_data *vc, char *cp) +static noinline_if_stackbloat void puts_queue(struct vc_data *vc, char *cp)
I don't see why, it's only called once in the callers. k_fn, however, is called several times in k_pad, so that could be why, but then it's rather be the inlining of k_fn which is a bad idea.
-static void fn_send_intr(struct vc_data *vc) +static noinline_if_stackbloat void fn_send_intr(struct vc_data *vc)
This one is only referenced, not called, I don't see how that could pose problem. Samuel