Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1667683

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

Show all headers | View raw

On Thu, Jun 15, 2017 at 6:53 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Thu, Jun 15, 2017 at 06:52:21AM +0200, Greg Kroah-Hartman wrote:
>> On Wed, Jun 14, 2017 at 11:15:38PM +0200, Arnd Bergmann 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.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > ---
>> >  drivers/tty/vt/keyboard.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
>> > index f4166263bb3a..c0d111444a0e 100644
>> > --- 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);
>> >  }
>>
>> Ugh, really?  We have to start telling gcc not to be stupid here?
>> That's not going to be easy, and will just entail us doing this all over
>> the place, right?
>>
>> The code isn't asking to be inlined, so why is gcc allowing it to be
>> done that way?  Doesn't that imply gcc is the problem here?
>
> Wait, you are now, in this patch, _asking_ for it to be inlined.  How is
> that solving anything?

The three functions that gain the attribute are all those that gcc decided
to inline for itself. Usually gcc makes reasonable inlining decisions, so
I left the existing behavior my marking them as 'inline' without
CONFIG_KASAN and 'noinline' when KASAN is enabled.

Would you rather see this patch instead?

diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index c28dd523f96e..25348c5ffcb7 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -13,8 +13,8 @@ extern int tty_prepare_flip_string(struct tty_port *port,
 extern void tty_flip_buffer_push(struct tty_port *port);
 void tty_schedule_flip(struct tty_port *port);

-static inline int tty_insert_flip_char(struct tty_port *port,
-                                       unsigned char ch, char flag)
+static noinline_if_stackbloat int
+tty_insert_flip_char(struct tty_port *port, unsigned char ch, char flag)
 {
        struct tty_buffer *tb = port->buf.tail;
        int change;

This is just as good at eliminating the crazy stack usage in vt/keyboard.o,
but it will also impact all other users of that function.

        Arnd

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 00/11] bring back stack frame warning with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:20 +0200
  [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:20 +0200
    Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Samuel Thibault <samuel.thibault@ens-lyon.org> - 2017-06-14 23:30 +0200
      Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-15 00:00 +0200
        Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Samuel Thibault <samuel.thibault@ens-lyon.org> - 2017-06-15 00:20 +0200
    Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-15 07:00 +0200
      Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-15 07:00 +0200
        Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-16 14:10 +0200
          Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-16 15:10 +0200
            Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-16 17:50 +0200
              Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Samuel Thibault <samuel.thibault@ens-lyon.org> - 2017-06-16 18:00 +0200
                Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-06-16 19:30 +0200
                Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-16 23:00 +0200
                Re: [PATCH v2 03/11] tty: kbd: reduce stack size with KASAN Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-06-16 23:10 +0200
  [PATCH v2 04/11] rocker: mark rocker_tlv_put_* functions as noinline_if_stackbloat Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:20 +0200
  [PATCH v2 01/11] compiler: introduce noinline_if_stackbloat annotation Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:20 +0200
  [PATCH v2 09/11] brcmsmac: split up wlc_phy_workarounds_nphy Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200
  [PATCH v2 06/11] dvb-frontends: reduce stack size in i2c access Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200
  [PATCH v2 11/11] kasan: rework Kconfig settings Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200
    Re: [PATCH v2 11/11] kasan: rework Kconfig settings Dmitry Vyukov <dvyukov@google.com> - 2017-06-15 09:10 +0200
      Re: [PATCH v2 11/11] kasan: rework Kconfig settings Dmitry Vyukov <dvyukov@google.com> - 2017-06-16 13:50 +0200
      Re: [PATCH v2 11/11] kasan: rework Kconfig settings Arnd Bergmann <arnd@arndb.de> - 2017-06-16 13:50 +0200
  [PATCH v2 08/11] brcmsmac: make some local variables 'static const' to reduce stack size Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200
    Re: [PATCH v2 08/11] brcmsmac: make some local variables 'static const' to reduce stack size Kalle Valo <kvalo@codeaurora.org> - 2017-06-15 17:00 +0200
  [PATCH v2 07/11] r820t: mark register functions as noinline_if_stackbloat Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200
  [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN Arnd Bergmann <arnd@arndb.de> - 2017-06-14 23:30 +0200

csiph-web