Date:   Tue, 2 Mar 2021 14:49:42 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Vlastimil Babka <vbabka@...e.cz>, Petr Mladek <pmladek@...e.com>
Cc:     Marco Elver <elver@...gle.com>, Timur Tabi <timur@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Matthew Wilcox <willy@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        roman.fietze@...na.com, Kees Cook <keescook@...omium.org>,
        John Ogness <john.ogness@...utronix.de>,
        Akinobu Mita <akinobu.mita@...il.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Pavel Machek <pavel@....cz>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux MM <linux-mm@...ck.org>
Subject: Re: [PATCH 3/3] [v4] lib/vsprintf: no_hash_pointers prints all
 addresses as unhashed

Hi Vlastimil, Petr,

On Tue, Mar 2, 2021 at 2:37 PM Vlastimil Babka <vbabka@...e.cz> wrote:
> On 3/2/21 2:29 PM, Petr Mladek wrote:
> > On Tue 2021-03-02 13:51:35, Geert Uytterhoeven wrote:
> >> > > > +
> >> > > > +       pr_warn("**********************************************************\n");
> >> > > > +       pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
> >> > > > +       pr_warn("**                                                      **\n");
> >> > > > +       pr_warn("** This system shows unhashed kernel memory addresses   **\n");
> >> > > > +       pr_warn("** via the console, logs, and other interfaces. This    **\n");
> >> > > > +       pr_warn("** might reduce the security of your system.            **\n");
> >> > > > +       pr_warn("**                                                      **\n");
> >> > > > +       pr_warn("** If you see this message and you are not debugging    **\n");
> >> > > > +       pr_warn("** the kernel, report this immediately to your system   **\n");
> >> > > > +       pr_warn("** administrator!                                       **\n");
> >> > > > +       pr_warn("**                                                      **\n");
> >> > > > +       pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
> >> > > > +       pr_warn("**********************************************************\n");
> >> > > > +
> >> > > > +       return 0;
> >> > > > +}
> >> > > > +early_param("no_hash_pointers", no_hash_pointers_enable);
> >> > >
> >> > > While bloat-o-meter is not smart enough to notice the real size impact,
> >> > > this does add more than 500 bytes of string data to the kernel.
> >> > > Do we really need such a large message?
> >> > > Perhaps the whole no_hash_pointers machinery should be protected by
> >> > > "#ifdef CONFIG_DEBUG_KERNEL"?
> >
> > This was the deal. The configure option is a no-go, see below and also
> > https://lore.kernel.org/r/CAHk-=wgaK4cz=K-JB4p-KPXBV73m9bja2w1W1Lr3iu8+NEPk7A@mail.gmail.com
>
> I think it's a no-go only when enabling such option equals to "no_hash_pointers"
> being always passed. What Geert suggests is that you need both
> CONFIG_DEBUG_KERNEL *and* no_hash_pointers and that's different.

Exactly.

> >> > We recently stumbled across this, and it appears an increasing number
> >> > of production kernels enable CONFIG_DEBUG_KERNEL [1], so it likely
> >> > isn't the solution (we tried to use CONFIG_DEBUG_KERNEL in similar
> >>
> >> I guess the people who do care about kernel size do know to disable
> >> CONFIG_DEBUG_KERNEL, so it would help them.
> >> The everything-but-the-kitchen-sink distro people don't care about kernel
> >> size anyway.
> >
> > The problem with the configure option is not about size. The problem is
> > that there would be many kernels in the wild with this option enabled.
> > People would install them without knowing that they are less secure.
>
> Same as above.
>
> > Distros would need to provide both kernels. Well, they already do.
> > But it might be worse. Some distros might even want to enable it
> > by default.
> >
> > Also many bugs might be debugged without this option. Some bugs
> > are hard to reproduce and might be visible only on production
> > systems. It should be possible to enable this only when really
> > needed and the user must be aware of the risk.
>
> So this is basically a kernel tinyfication issue, right? Is that still pursued
> today? Are there better config options suitable for this than CONFIG_DEBUG_KERNEL?

As long as I hear about products running Linux on SoCs with 10 MiB of
SRAM, I think the answer is yes.
I'm not immediately aware of a better config option.  There are no more
TINY options left, and EXPERT selects DEBUG_KERNEL.

> >> > Would placing the strings into an __initconst array help?
> >>
> >> That would indeed help to reduce run-time memory consumption.
> >
> > Sure. We could do this. Do you want to send a patch, please?

Added to my list.

> >> It would not solve the raw kernel size increase.
> >
> > I see. Well, the compression should be pretty efficient
> > for a text (with many spaces).

My worry is not about the medium for storing the kernel image, but the
RAM where the kernel image is loaded.  The former is usually less
restricted in size, and easier to expand, than the latter,

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists