2021-03-31 10:01:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 0/3] Use pr_crit() instead of long fancy messages

Hi all,

While long fancy messages have a higher probability of being seen than
small messages, they may scroll of the screen fast, if visible at all,
and may still be missed. In addition, they increase boot time and
kernel size.

The correct mechanism to increase importance of a kernel message is not
to draw fancy boxes with more text, but to shout louder, i.e. increase
the message's reporting level. Making sure the administrator of the
system is aware of such a message is a system policy, and is the
responsability of a user-space log daemon.

This series increases the reporting level of such messages from
KERN_WARNING to KERN_CRIT, and removes irrelevant text and graphics.
It was made against linux-next, but applies to v5.12-rc5 with an offset
for the last patch.

Each of these patches reduces kernel size by ca. 0.5 KiB.

Thanks for your comments!

Geert Uytterhoeven (3):
iommu: Use pr_crit() instead of long fancy messages
tracing: Use pr_crit() instead of long fancy messages
lib/vsprintf: Use pr_crit() instead of long fancy messages

drivers/iommu/iommu-debugfs.c | 19 ++++---------------
kernel/trace/trace.c | 17 +++--------------
lib/vsprintf.c | 17 +++--------------
3 files changed, 10 insertions(+), 43 deletions(-)

--
2.25.1

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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


2021-03-31 10:01:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 3/3] lib/vsprintf: Use pr_crit() instead of long fancy messages

While long fancy messages have a higher probability of being seen than
small messages, they may scroll of the screen fast, if visible at all,
and may still be missed. In addition, they increase boot time and
kernel size.

The correct mechanism to increase importance of a kernel message is not
to draw fancy boxes with more text, but to shout louder, i.e. increase
the message's reporting level. Making sure the administrator of the
system is aware of such a message is a system policy, and is the
responsability of a user-space log daemon.

Fix this by increasing the reporting level from KERN_WARNING to
KERN_CRIT, and removing irrelevant text and graphics.

This reduces kernel size by ca. 0.5 KiB.

Fixes: 5ead723a20e0447b ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
lib/vsprintf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9b423359bb6433d3..0293f1b89064b287 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -2193,20 +2193,9 @@ static int __init no_hash_pointers_enable(char *str)

no_hash_pointers = true;

- 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");
-
+ pr_crit("This system shows unhashed kernel memory addresses\n");
+ pr_crit("via the console, logs, and other interfaces. This\n");
+ pr_crit("might reduce the security of your system.\n");
return 0;
}
early_param("no_hash_pointers", no_hash_pointers_enable);
--
2.25.1

2021-03-31 10:01:17

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 1/3] iommu: Use pr_crit() instead of long fancy messages

While long fancy messages have a higher probability of being seen than
small messages, they may scroll of the screen fast, if visible at all,
and may still be missed. In addition, they increase boot time and
kernel size.

The correct mechanism to increase importance of a kernel message is not
to draw fancy boxes with more text, but to shout louder, i.e. increase
the message's reporting level. Making sure the administrator of the
system is aware of such a message is a system policy, and is the
responsability of a user-space log daemon.

Fix this by increasing the reporting level from KERN_WARNING to
KERN_CRIT, and removing irrelevant text and graphics.

This reduces kernel size by ca. 0.5 KiB.

Fixes: bad614b24293ae46 ("iommu: Enable debugfs exposure of IOMMU driver internals")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/iommu/iommu-debugfs.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/iommu-debugfs.c b/drivers/iommu/iommu-debugfs.c
index f0354894209648fd..c3306998de0687bd 100644
--- a/drivers/iommu/iommu-debugfs.c
+++ b/drivers/iommu/iommu-debugfs.c
@@ -32,20 +32,9 @@ void iommu_debugfs_setup(void)
{
if (!iommu_debugfs_dir) {
iommu_debugfs_dir = debugfs_create_dir("iommu", NULL);
- pr_warn("\n");
- pr_warn("*************************************************************\n");
- pr_warn("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n");
- pr_warn("** **\n");
- pr_warn("** IOMMU DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL **\n");
- pr_warn("** **\n");
- pr_warn("** This means that this kernel is built to expose internal **\n");
- pr_warn("** IOMMU data structures, which may compromise security on **\n");
- pr_warn("** your system. **\n");
- pr_warn("** **\n");
- pr_warn("** If you see this message and you are not debugging the **\n");
- pr_warn("** kernel, report this immediately to your vendor! **\n");
- pr_warn("** **\n");
- pr_warn("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n");
- pr_warn("*************************************************************\n");
+ pr_crit("IOMMU DebugFS SUPPORT HAS BEEN ENABLED IN THIS KERNEL\n");
+ pr_crit("This means that this kernel is built to expose internal\n");
+ pr_crit("IOMMU data structures, which may compromise security on\n");
+ pr_crit("your system.\n");
}
}
--
2.25.1

2021-03-31 10:02:50

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH 2/3] tracing: Use pr_crit() instead of long fancy messages

While long fancy messages have a higher probability of being seen than
small messages, they may scroll of the screen fast, if visible at all,
and may still be missed. In addition, they increase boot time and
kernel size.

The correct mechanism to increase importance of a kernel message is not
to draw fancy boxes with more text, but to shout louder, i.e. increase
the message's reporting level. Making sure the administrator of the
system is aware of such a message is a system policy, and is the
responsability of a user-space log daemon.

Fix this by increasing the reporting level from KERN_WARNING to
KERN_CRIT, and removing irrelevant text and graphics.

This reduces kernel size by ca. 0.5 KiB.

Fixes: 2184db46e425c2b8 ("tracing: Print nasty banner when trace_printk() is in use")
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
kernel/trace/trace.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index eccb4e1187cc788e..b3a93aff01045923 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3175,20 +3175,9 @@ void trace_printk_init_buffers(void)

/* trace_printk() is for debug use only. Don't use it in production. */

- pr_warn("\n");
- pr_warn("**********************************************************\n");
- pr_warn("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n");
- pr_warn("** **\n");
- pr_warn("** trace_printk() being used. Allocating extra memory. **\n");
- pr_warn("** **\n");
- pr_warn("** This means that this is a DEBUG kernel and it is **\n");
- pr_warn("** unsafe for production use. **\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 vendor! **\n");
- pr_warn("** **\n");
- pr_warn("** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **\n");
- pr_warn("**********************************************************\n");
+ pr_crit("trace_printk() being used. Allocating extra memory.\n");
+ pr_crit("This means that this is a DEBUG kernel and it is\n");
+ pr_crit("unsafe for production use.\n");

/* Expand the buffers to set size */
tracing_update_buffers();
--
2.25.1

2021-03-31 13:41:50

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 2/3] tracing: Use pr_crit() instead of long fancy messages

On Wed, 31 Mar 2021 11:31:03 +0200
Geert Uytterhoeven <[email protected]> wrote:

> This reduces kernel size by ca. 0.5 KiB.

If you are worried about size, disable tracing and it will go away
entirely. 0.5KiB is a drop in the bucket compared to what tracing adds in
size overhead.

Sorry, but NAK.

This has been very successful in stopping people from adding trace_printk()
to the kernel, and I like to keep it that way.

-- Steve

2021-04-01 09:41:08

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/3] tracing: Use pr_crit() instead of long fancy messages

Hi Steven,

On Wed, Mar 31, 2021 at 3:40 PM Steven Rostedt <[email protected]> wrote:
> On Wed, 31 Mar 2021 11:31:03 +0200
> Geert Uytterhoeven <[email protected]> wrote:
>
> > This reduces kernel size by ca. 0.5 KiB.
>
> If you are worried about size, disable tracing and it will go away
> entirely. 0.5KiB is a drop in the bucket compared to what tracing adds in
> size overhead.

Fair enough for this particular case, as tracing can be disabled.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-04-01 18:47:51

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH 2/3] tracing: Use pr_crit() instead of long fancy messages

On 2021-04-01 10:39, Geert Uytterhoeven wrote:
> Hi Steven,
>
> On Wed, Mar 31, 2021 at 3:40 PM Steven Rostedt <[email protected]> wrote:
>> On Wed, 31 Mar 2021 11:31:03 +0200
>> Geert Uytterhoeven <[email protected]> wrote:
>>
>>> This reduces kernel size by ca. 0.5 KiB.
>>
>> If you are worried about size, disable tracing and it will go away
>> entirely. 0.5KiB is a drop in the bucket compared to what tracing adds in
>> size overhead.
>
> Fair enough for this particular case, as tracing can be disabled.

I think the same argument can be applied to patch #1 - it's hard to
imaging anyone debugging an IOMMU driver on a system where a few hundred
bytes makes the slightest bit of difference, and for people not
debugging IOMMU drivers it should be moot (per the message itself).

Robin.

2021-05-17 06:41:11

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 3/3] lib/vsprintf: Use pr_crit() instead of long fancy messages

On Wed, Mar 31, 2021 at 11:59 AM Geert Uytterhoeven
<[email protected]> wrote:
> While long fancy messages have a higher probability of being seen than
> small messages, they may scroll of the screen fast, if visible at all,
> and may still be missed. In addition, they increase boot time and
> kernel size.
>
> The correct mechanism to increase importance of a kernel message is not
> to draw fancy boxes with more text, but to shout louder, i.e. increase
> the message's reporting level. Making sure the administrator of the
> system is aware of such a message is a system policy, and is the
> responsability of a user-space log daemon.
>
> Fix this by increasing the reporting level from KERN_WARNING to
> KERN_CRIT, and removing irrelevant text and graphics.
>
> This reduces kernel size by ca. 0.5 KiB.
>
> Fixes: 5ead723a20e0447b ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed")
> Signed-off-by: Geert Uytterhoeven <[email protected]>

No comments?
Unlike the cases handled by the other two patches in this series,
this one cannot be configured out.

Thanks!

> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -2193,20 +2193,9 @@ static int __init no_hash_pointers_enable(char *str)
>
> no_hash_pointers = true;
>
> - 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");
> -
> + pr_crit("This system shows unhashed kernel memory addresses\n");
> + pr_crit("via the console, logs, and other interfaces. This\n");
> + pr_crit("might reduce the security of your system.\n");
> return 0;
> }
> early_param("no_hash_pointers", no_hash_pointers_enable);

Gr{oetje,eeting}s,

Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2021-05-20 12:54:49

by Petr Mladek

[permalink] [raw]
Subject: Re: [PATCH 3/3] lib/vsprintf: Use pr_crit() instead of long fancy messages

On Mon 2021-05-17 08:21:12, Geert Uytterhoeven wrote:
> On Wed, Mar 31, 2021 at 11:59 AM Geert Uytterhoeven
> <[email protected]> wrote:
> > While long fancy messages have a higher probability of being seen than
> > small messages, they may scroll of the screen fast, if visible at all,
> > and may still be missed. In addition, they increase boot time and
> > kernel size.
> >
> > The correct mechanism to increase importance of a kernel message is not
> > to draw fancy boxes with more text, but to shout louder, i.e. increase
> > the message's reporting level. Making sure the administrator of the
> > system is aware of such a message is a system policy, and is the
> > responsability of a user-space log daemon.
> >
> > Fix this by increasing the reporting level from KERN_WARNING to
> > KERN_CRIT, and removing irrelevant text and graphics.
> >
> > This reduces kernel size by ca. 0.5 KiB.
> >
> > Fixes: 5ead723a20e0447b ("lib/vsprintf: no_hash_pointers prints all addresses as unhashed")
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> No comments?
> Unlike the cases handled by the other two patches in this series,
> this one cannot be configured out.

IMHO, the best solution would be to create a generic API for
eye-catching messages.

I am sure that WARN() is misused on many locations all over the kernel
because people just wanted eye-catching message, for example, see
https://lore.kernel.org/r/[email protected]

It might be a win-win solution.

Best Regards,
Petr