[PATCH 0/3] Use pr_crit() instead of long fancy messages

[PATCH 0/3] Use pr_crit() instead of long fancy messages

[Geert Uytterhoeven]

	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 -- geert@linux-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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Geert Uytterhoeven]

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 <geert+renesas@glider.be>
---
 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Geert Uytterhoeven]

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 <geert+renesas@glider.be>
---
 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Steven Rostedt]

On Wed, 31 Mar 2021 11:31:03 +0200
Geert Uytterhoeven <geert+renesas@glider.be> 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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Petr Mladek via iommu]

On Wed 2021-03-31 09:40:07, Steven Rostedt wrote:
> On Wed, 31 Mar 2021 11:31:03 +0200
> Geert Uytterhoeven <geert+renesas@glider.be> 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.

I agree with Steven. I believe that the eye-catching form is
important.

Anyway, all three patches are replacing text that have
many common parts. It is:

	pr_warn("\n");
	pr_warn("**********************************************************\n");
	pr_warn("**   NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE   **\n");
	pr_warn("**                                                      **\n");
	pr_warn("** <Something is enabled/used.>			 **\n");
	pr_warn("**                                                      **\n");
	pr_warn("** <Description of the effect and why it should not     **\n");
	pr_warn("** happen on production kernel>			**\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");

It might be useful to avoid cut&pasting this entire blob and unify the
common parts.

My only concern is how to define the caller-specific lines so that
they fit with the common ones. They should not be longer than the
header and footer. We need to find a good compromise between
usability and over-engineering.

Also we have to keep bikeshading under control ;-)

Best Regards,
Petr
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Geert Uytterhoeven]

Hi Steven,

On Wed, Mar 31, 2021 at 3:40 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 31 Mar 2021 11:31:03 +0200
> Geert Uytterhoeven <geert+renesas@glider.be> 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 -- geert@linux-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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Robin Murphy]

On 2021-04-01 10:39, Geert Uytterhoeven wrote:
> Hi Steven,
> 
> On Wed, Mar 31, 2021 at 3:40 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Wed, 31 Mar 2021 11:31:03 +0200
>> Geert Uytterhoeven <geert+renesas@glider.be> 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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Geert Uytterhoeven]

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 <geert+renesas@glider.be>
---
 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

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Geert Uytterhoeven]

On Wed, Mar 31, 2021 at 11:59 AM Geert Uytterhoeven
<geert+renesas@glider.be> 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 <geert+renesas@glider.be>

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 -- geert@linux-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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Petr Mladek via iommu]

On Mon 2021-05-17 08:21:12, Geert Uytterhoeven wrote:
> On Wed, Mar 31, 2021 at 11:59 AM Geert Uytterhoeven
> <geert+renesas@glider.be> 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 <geert+renesas@glider.be>
> 
> 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/2149df3f542d25ce15d049e81d6188bb7198478c.camel@fi.rohmeurope.com

It might be a win-win solution.

Best Regards,
Petr
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu