linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line
@ 2018-09-29 16:45 zhe.he
  2018-09-29 16:45 ` [PATCH v5 2/4] printk: Correct wrong casting zhe.he
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: zhe.he @ 2018-09-29 16:45 UTC (permalink / raw)
  To: pmladek, sergey.senozhatsky, rostedt, linux-kernel, zhe.he

From: He Zhe <zhe.he@windriver.com>

log_buf_len_setup does not check input argument before passing it to
simple_strtoull. The argument would be a NULL pointer if "log_buf_len",
without its value, is set in command line and thus causes the following
panic.

PANIC: early exception 0xe3 IP 10:ffffffffaaeacd0d error 0 cr2 0x0
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc4-yocto-standard+ #1
[    0.000000] RIP: 0010:_parse_integer_fixup_radix+0xd/0x70
...
[    0.000000] Call Trace:
[    0.000000]  simple_strtoull+0x29/0x70
[    0.000000]  memparse+0x26/0x90
[    0.000000]  log_buf_len_setup+0x17/0x22
[    0.000000]  do_early_param+0x57/0x8e
[    0.000000]  parse_args+0x208/0x320
[    0.000000]  ? rdinit_setup+0x30/0x30
[    0.000000]  parse_early_options+0x29/0x2d
[    0.000000]  ? rdinit_setup+0x30/0x30
[    0.000000]  parse_early_param+0x36/0x4d
[    0.000000]  setup_arch+0x336/0x99e
[    0.000000]  start_kernel+0x6f/0x4ee
[    0.000000]  x86_64_start_reservations+0x24/0x26
[    0.000000]  x86_64_start_kernel+0x6f/0x72
[    0.000000]  secondary_startup_64+0xa4/0xb0

This patch adds a check to prevent the panic.

Signed-off-by: He Zhe <zhe.he@windriver.com>
Cc: stable@vger.kernel.org
Cc: pmladek@suse.com
Cc: sergey.senozhatsky@gmail.com
Cc: rostedt@goodmis.org
---
v2:
Split out the addition of pr_fmt and the unsigned update
v3:
Remove error message for NULL pointer
Add check and error message for over 4G use
v4:
Split each piece into one patch
v5:
Remove a redundant print prefix

 kernel/printk/printk.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 9bf5404..06045ab 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1048,7 +1048,12 @@ static void __init log_buf_len_update(unsigned size)
 /* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
-	unsigned size = memparse(str, &str);
+	unsigned int size;
+
+	if (!str)
+		return -EINVAL;
+
+	size = memparse(str, &str);
 
 	log_buf_len_update(size);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/4] printk: Correct wrong casting
  2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
@ 2018-09-29 16:45 ` zhe.he
  2018-10-02  5:50   ` Sergey Senozhatsky
  2018-09-29 16:45 ` [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix zhe.he
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: zhe.he @ 2018-09-29 16:45 UTC (permalink / raw)
  To: pmladek, sergey.senozhatsky, rostedt, linux-kernel, zhe.he

From: He Zhe <zhe.he@windriver.com>

Correct wrong casting that might cut off the normal output.

Signed-off-by: He Zhe <zhe.he@windriver.com>
Cc: stable@vger.kernel.org
Cc: pmladek@suse.com
Cc: sergey.senozhatsky@gmail.com
Cc: rostedt@goodmis.org
---
 kernel/printk/printk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 06045ab..c9a0be3 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2356,8 +2356,9 @@ void console_unlock(void)
 		printk_safe_enter_irqsave(flags);
 		raw_spin_lock(&logbuf_lock);
 		if (console_seq < log_first_seq) {
-			len = sprintf(text, "** %u printk messages dropped **\n",
-				      (unsigned)(log_first_seq - console_seq));
+			len = sprintf(text,
+				      "** %llu printk messages dropped **\n",
+				      log_first_seq - console_seq);
 
 			/* messages are gone, move to first one */
 			console_seq = log_first_seq;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix
  2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
  2018-09-29 16:45 ` [PATCH v5 2/4] printk: Correct wrong casting zhe.he
@ 2018-09-29 16:45 ` zhe.he
  2018-10-02  5:52   ` Sergey Senozhatsky
  2018-10-08 13:04   ` Petr Mladek
  2018-09-29 16:45 ` [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G zhe.he
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 18+ messages in thread
From: zhe.he @ 2018-09-29 16:45 UTC (permalink / raw)
  To: pmladek, sergey.senozhatsky, rostedt, linux-kernel, zhe.he

From: He Zhe <zhe.he@windriver.com>

Add KBUILD_MODNAME to make prints more clear.

Signed-off-by: He Zhe <zhe.he@windriver.com>
Cc: pmladek@suse.com
Cc: sergey.senozhatsky@gmail.com
Cc: rostedt@goodmis.org
---
 kernel/printk/printk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index c9a0be3..0f24d7f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -16,6 +16,8 @@
  *	01Mar01 Andrew Morton
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/tty.h>
@@ -2695,7 +2697,7 @@ void register_console(struct console *newcon)
 
 	if (newcon->flags & CON_EXTENDED)
 		if (!nr_ext_console_drivers++)
-			pr_info("printk: continuation disabled due to ext consoles, expect more fragments in /dev/kmsg\n");
+			pr_info("continuation disabled due to ext consoles, expect more fragments in /dev/kmsg\n");
 
 	if (newcon->flags & CON_PRINTBUFFER) {
 		/*
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
  2018-09-29 16:45 ` [PATCH v5 2/4] printk: Correct wrong casting zhe.he
  2018-09-29 16:45 ` [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix zhe.he
@ 2018-09-29 16:45 ` zhe.he
  2018-10-02  5:51   ` Sergey Senozhatsky
  2018-10-08 13:59   ` Petr Mladek
  2018-10-02  5:48 ` [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line Sergey Senozhatsky
  2018-10-08 13:01 ` Petr Mladek
  4 siblings, 2 replies; 18+ messages in thread
From: zhe.he @ 2018-09-29 16:45 UTC (permalink / raw)
  To: pmladek, sergey.senozhatsky, rostedt, linux-kernel, zhe.he

From: He Zhe <zhe.he@windriver.com>

Give explicit error for users who want to use larger log buffer.

Signed-off-by: He Zhe <zhe.he@windriver.com>
Cc: pmladek@suse.com
Cc: sergey.senozhatsky@gmail.com
Cc: rostedt@goodmis.org
---
 kernel/printk/printk.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b84aac0..5ccfd5d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1039,18 +1039,23 @@ void log_buf_vmcoreinfo_setup(void)
 static unsigned long __initdata new_log_buf_len;
 
 /* we practice scaling the ring buffer by powers of 2 */
-static void __init log_buf_len_update(unsigned size)
+static void __init log_buf_len_update(u64 size)
 {
+	if (size > UINT_MAX) {
+		size = UINT_MAX;
+		pr_err("log_buf over 4G is not supported.\n");
+	}
+
 	if (size)
 		size = roundup_pow_of_two(size);
 	if (size > log_buf_len)
-		new_log_buf_len = size;
+		new_log_buf_len = (unsigned long)size;
 }
 
 /* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
-	unsigned int size;
+	u64 size;
 
 	if (!str)
 		return -EINVAL;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line
  2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
                   ` (2 preceding siblings ...)
  2018-09-29 16:45 ` [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G zhe.he
@ 2018-10-02  5:48 ` Sergey Senozhatsky
  2018-10-08 13:01 ` Petr Mladek
  4 siblings, 0 replies; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-02  5:48 UTC (permalink / raw)
  To: zhe.he; +Cc: pmladek, sergey.senozhatsky, rostedt, linux-kernel

On (09/30/18 00:45), zhe.he@windriver.com wrote:
> 
> This patch adds a check to prevent the panic.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> Cc: stable@vger.kernel.org
> Cc: pmladek@suse.com
> Cc: sergey.senozhatsky@gmail.com
> Cc: rostedt@goodmis.org

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/4] printk: Correct wrong casting
  2018-09-29 16:45 ` [PATCH v5 2/4] printk: Correct wrong casting zhe.he
@ 2018-10-02  5:50   ` Sergey Senozhatsky
  2018-10-08 13:03     ` Petr Mladek
  0 siblings, 1 reply; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-02  5:50 UTC (permalink / raw)
  To: zhe.he; +Cc: pmladek, sergey.senozhatsky, rostedt, linux-kernel

On (09/30/18 00:45), zhe.he@windriver.com wrote:
> Correct wrong casting that might cut off the normal output.

A commit message probably could have been a bit more descriptive,
mentioning that log_first_seq and console_seq are 64-bit unsigned
integers, this is a Cc material potentially.

> Signed-off-by: He Zhe <zhe.he@windriver.com>
> Cc: stable@vger.kernel.org
> Cc: pmladek@suse.com
> Cc: sergey.senozhatsky@gmail.com
> Cc: rostedt@goodmis.org

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-09-29 16:45 ` [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G zhe.he
@ 2018-10-02  5:51   ` Sergey Senozhatsky
  2018-10-08 13:59   ` Petr Mladek
  1 sibling, 0 replies; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-02  5:51 UTC (permalink / raw)
  To: zhe.he; +Cc: pmladek, sergey.senozhatsky, rostedt, linux-kernel

On (09/30/18 00:45), zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> Give explicit error for users who want to use larger log buffer.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> Cc: pmladek@suse.com
> Cc: sergey.senozhatsky@gmail.com
> Cc: rostedt@goodmis.org

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
So people will know who to blame ;)

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix
  2018-09-29 16:45 ` [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix zhe.he
@ 2018-10-02  5:52   ` Sergey Senozhatsky
  2018-10-08 13:04   ` Petr Mladek
  1 sibling, 0 replies; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-02  5:52 UTC (permalink / raw)
  To: zhe.he; +Cc: pmladek, sergey.senozhatsky, rostedt, linux-kernel

On (09/30/18 00:45), zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> Add KBUILD_MODNAME to make prints more clear.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> Cc: pmladek@suse.com
> Cc: sergey.senozhatsky@gmail.com
> Cc: rostedt@goodmis.org

Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line
  2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
                   ` (3 preceding siblings ...)
  2018-10-02  5:48 ` [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line Sergey Senozhatsky
@ 2018-10-08 13:01 ` Petr Mladek
  4 siblings, 0 replies; 18+ messages in thread
From: Petr Mladek @ 2018-10-08 13:01 UTC (permalink / raw)
  To: zhe.he; +Cc: sergey.senozhatsky, rostedt, linux-kernel

On Sun 2018-09-30 00:45:50, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> log_buf_len_setup does not check input argument before passing it to
> simple_strtoull. The argument would be a NULL pointer if "log_buf_len",
> without its value, is set in command line and thus causes the following
> panic.
> 
> PANIC: early exception 0xe3 IP 10:ffffffffaaeacd0d error 0 cr2 0x0
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.19.0-rc4-yocto-standard+ #1
> [    0.000000] RIP: 0010:_parse_integer_fixup_radix+0xd/0x70
> ...
> [    0.000000] Call Trace:
> [    0.000000]  simple_strtoull+0x29/0x70
> [    0.000000]  memparse+0x26/0x90
> [    0.000000]  log_buf_len_setup+0x17/0x22
> [    0.000000]  do_early_param+0x57/0x8e
> [    0.000000]  parse_args+0x208/0x320
> [    0.000000]  ? rdinit_setup+0x30/0x30
> [    0.000000]  parse_early_options+0x29/0x2d
> [    0.000000]  ? rdinit_setup+0x30/0x30
> [    0.000000]  parse_early_param+0x36/0x4d
> [    0.000000]  setup_arch+0x336/0x99e
> [    0.000000]  start_kernel+0x6f/0x4ee
> [    0.000000]  x86_64_start_reservations+0x24/0x26
> [    0.000000]  x86_64_start_kernel+0x6f/0x72
> [    0.000000]  secondary_startup_64+0xa4/0xb0
> 
> This patch adds a check to prevent the panic.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>

JFYI, I have pushed this patch into printk.git, branch for-4.20.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/4] printk: Correct wrong casting
  2018-10-02  5:50   ` Sergey Senozhatsky
@ 2018-10-08 13:03     ` Petr Mladek
  0 siblings, 0 replies; 18+ messages in thread
From: Petr Mladek @ 2018-10-08 13:03 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: zhe.he, sergey.senozhatsky, rostedt, linux-kernel

On Tue 2018-10-02 14:50:18, Sergey Senozhatsky wrote:
> On (09/30/18 00:45), zhe.he@windriver.com wrote:

> > Correct wrong casting that might cut off the normal output.
> 
> A commit message probably could have been a bit more descriptive,
> mentioning that log_first_seq and console_seq are 64-bit unsigned
> integers, this is a Cc material potentially.

I have used the following text:

log_first_seq and console_seq are 64-bit unsigned integers.
Correct a wrong casting that might cut off the output. 

> > Signed-off-by: He Zhe <zhe.he@windriver.com>
> > Cc: stable@vger.kernel.org
> > Cc: pmladek@suse.com
> > Cc: sergey.senozhatsky@gmail.com
> > Cc: rostedt@goodmis.org
> 
> Reviewed-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>

And pushed this patch into printk.git, branch for-4.20.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix
  2018-09-29 16:45 ` [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix zhe.he
  2018-10-02  5:52   ` Sergey Senozhatsky
@ 2018-10-08 13:04   ` Petr Mladek
  1 sibling, 0 replies; 18+ messages in thread
From: Petr Mladek @ 2018-10-08 13:04 UTC (permalink / raw)
  To: zhe.he; +Cc: sergey.senozhatsky, rostedt, linux-kernel

On Sun 2018-09-30 00:45:52, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> Add KBUILD_MODNAME to make prints more clear.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>

JFYI, I have pushed this patch into printk.git, for-4.20 branch.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-09-29 16:45 ` [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G zhe.he
  2018-10-02  5:51   ` Sergey Senozhatsky
@ 2018-10-08 13:59   ` Petr Mladek
  2018-10-08 14:59     ` Sergey Senozhatsky
  2018-10-08 16:30     ` He Zhe
  1 sibling, 2 replies; 18+ messages in thread
From: Petr Mladek @ 2018-10-08 13:59 UTC (permalink / raw)
  To: zhe.he; +Cc: sergey.senozhatsky, rostedt, linux-kernel

On Sun 2018-09-30 00:45:53, zhe.he@windriver.com wrote:
> From: He Zhe <zhe.he@windriver.com>
> 
> Give explicit error for users who want to use larger log buffer.
> 
> Signed-off-by: He Zhe <zhe.he@windriver.com>
> Cc: pmladek@suse.com
> Cc: sergey.senozhatsky@gmail.com
> Cc: rostedt@goodmis.org
> ---
>  kernel/printk/printk.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index b84aac0..5ccfd5d 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1039,18 +1039,23 @@ void log_buf_vmcoreinfo_setup(void)
>  static unsigned long __initdata new_log_buf_len;
>  
>  /* we practice scaling the ring buffer by powers of 2 */
> -static void __init log_buf_len_update(unsigned size)
> +static void __init log_buf_len_update(u64 size)
>  {
> +	if (size > UINT_MAX) {
> +		size = UINT_MAX;
> +		pr_err("log_buf over 4G is not supported.\n");

I tried this patch with log_buf_len=5G. The kernel did not crash
but dmesg shown some mess. There are several 32-bit variables
to store the size, for example:

static u32 log_buf_len = __LOG_BUF_LEN;
u32 log_buf_len_get(void)
static u32 log_first_idx;
static u32 log_next_idx;

I guess that the code somewhere does not detect an overflows
correctly.

I am not motivated enought to add support for such huge message
buffer. Therefore I suggest to limit it to 32G for now.

This patch worked for me:


From d63b781b596ccb3d205801b2ba944797fa7ab0ce Mon Sep 17 00:00:00 2001
From: He Zhe <zhe.he@windriver.com>
Date: Sun, 30 Sep 2018 00:45:53 +0800
Subject: [PATCH] printk: Give error on attempt to set log buffer length to
 over 2G

The current printk() is ready to handle log buffer size up to 2G.
Give an explicit error for users who want to use larger log buffer.

Also fix printk formatting to show the 2G as a positive number.

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: He Zhe <zhe.he@windriver.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 15f3e70be448..f595107ddf42 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1040,18 +1040,23 @@ void log_buf_vmcoreinfo_setup(void)
 static unsigned long __initdata new_log_buf_len;
 
 /* we practice scaling the ring buffer by powers of 2 */
-static void __init log_buf_len_update(unsigned size)
+static void __init log_buf_len_update(u64 size)
 {
+	if (size > UINT_MAX) {
+		size = UINT_MAX;
+		pr_err("log_buf over 2G is not supported.\n");
+	}
+
 	if (size)
 		size = roundup_pow_of_two(size);
 	if (size > log_buf_len)
-		new_log_buf_len = size;
+		new_log_buf_len = (unsigned long)size;
 }
 
 /* save requested log_buf_len since it's too early to process it */
 static int __init log_buf_len_setup(char *str)
 {
-	unsigned int size;
+	u64 size;
 
 	if (!str)
 		return -EINVAL;
@@ -1121,7 +1126,7 @@ void __init setup_log_buf(int early)
 	}
 
 	if (unlikely(!new_log_buf)) {
-		pr_err("log_buf_len: %ld bytes not available\n",
+		pr_err("log_buf_len: %lu bytes not available\n",
 			new_log_buf_len);
 		return;
 	}
@@ -1134,8 +1139,8 @@ void __init setup_log_buf(int early)
 	memcpy(log_buf, __log_buf, __LOG_BUF_LEN);
 	logbuf_unlock_irqrestore(flags);
 
-	pr_info("log_buf_len: %d bytes\n", log_buf_len);
-	pr_info("early log buf free: %d(%d%%)\n",
+	pr_info("log_buf_len: %u bytes\n", log_buf_len);
+	pr_info("early log buf free: %u(%u%%)\n",
 		free, (free * 100) / __LOG_BUF_LEN);
 }
 
-- 
2.13.7


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-08 13:59   ` Petr Mladek
@ 2018-10-08 14:59     ` Sergey Senozhatsky
  2018-10-09 13:05       ` Petr Mladek
  2018-10-08 16:30     ` He Zhe
  1 sibling, 1 reply; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-08 14:59 UTC (permalink / raw)
  To: Petr Mladek; +Cc: zhe.he, sergey.senozhatsky, rostedt, linux-kernel

On (10/08/18 15:59), Petr Mladek wrote:
> I tried this patch with log_buf_len=5G. The kernel did not crash
> but dmesg shown some mess. There are several 32-bit variables
> to store the size, for example:
> 
> static u32 log_buf_len = __LOG_BUF_LEN;
> u32 log_buf_len_get(void)
> static u32 log_first_idx;
> static u32 log_next_idx;
> 
> I guess that the code somewhere does not detect an overflows
> correctly.
> 
> I am not motivated enought to add support for such huge message
> buffer. Therefore I suggest to limit it to 32G for now.

Yeah, I think we gonna have problems even with a 4G logbuf and a 32-bit
user-space doing syslog(int len).

I agree on the "not motivated enough" part ;)

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-08 13:59   ` Petr Mladek
  2018-10-08 14:59     ` Sergey Senozhatsky
@ 2018-10-08 16:30     ` He Zhe
  1 sibling, 0 replies; 18+ messages in thread
From: He Zhe @ 2018-10-08 16:30 UTC (permalink / raw)
  To: Petr Mladek; +Cc: sergey.senozhatsky, rostedt, linux-kernel



On 2018年10月08日 21:59, Petr Mladek wrote:
> I tried this patch with log_buf_len=5G. The kernel did not crash
> but dmesg shown some mess. There are several 32-bit variables
> to store the size, for example:
>
> static u32 log_buf_len = __LOG_BUF_LEN;
> u32 log_buf_len_get(void)
> static u32 log_first_idx;
> static u32 log_next_idx;
>
> I guess that the code somewhere does not detect an overflows
> correctly.
>
> I am not motivated enought to add support for such huge message
> buffer. Therefore I suggest to limit it to 32G for now.

I'm also fine with this suggestion. Thanks.

Zhe


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-08 14:59     ` Sergey Senozhatsky
@ 2018-10-09 13:05       ` Petr Mladek
  2018-10-09 13:57         ` Sergey Senozhatsky
  0 siblings, 1 reply; 18+ messages in thread
From: Petr Mladek @ 2018-10-09 13:05 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: zhe.he, rostedt, linux-kernel

On Mon 2018-10-08 23:59:50, Sergey Senozhatsky wrote:
> On (10/08/18 15:59), Petr Mladek wrote:
> > I tried this patch with log_buf_len=5G. The kernel did not crash
> > but dmesg shown some mess. There are several 32-bit variables
> > to store the size, for example:
> > 
> > static u32 log_buf_len = __LOG_BUF_LEN;
> > u32 log_buf_len_get(void)
> > static u32 log_first_idx;
> > static u32 log_next_idx;
> > 
> > I guess that the code somewhere does not detect an overflows
> > correctly.
> > 
> > I am not motivated enought to add support for such huge message
> > buffer. Therefore I suggest to limit it to 32G for now.
> 
> Yeah, I think we gonna have problems even with a 4G logbuf and a 32-bit
> user-space doing syslog(int len).
> 
> I agree on the "not motivated enough" part ;)

OK, I have pushed an updated patch that has the limit 2GB
into printk.git, for-4.20 branch.

Note that it is slightly different than the yesterday's proposal.
I made a mistake in testing and still compared with UNIT_MAX.

The pushed version can be seen at
https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.20&id=e6fe3e5b7d16e8f146a4ae7fe481bc6e97acde1e

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-09 13:05       ` Petr Mladek
@ 2018-10-09 13:57         ` Sergey Senozhatsky
  2018-10-10  8:09           ` Petr Mladek
  0 siblings, 1 reply; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-09 13:57 UTC (permalink / raw)
  To: Petr Mladek; +Cc: Sergey Senozhatsky, zhe.he, rostedt, linux-kernel

On (10/09/18 15:05), Petr Mladek wrote:
> > 
> > Yeah, I think we gonna have problems even with a 4G logbuf and a 32-bit
> > user-space doing syslog(int len).
> > 
> > I agree on the "not motivated enough" part ;)
> 
> OK, I have pushed an updated patch that has the limit 2GB
> into printk.git, for-4.20 branch.
> 
> Note that it is slightly different than the yesterday's proposal.
> I made a mistake in testing and still compared with UNIT_MAX.
> 
> The pushed version can be seen at
> https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.20&id=e6fe3e5b7d16e8f146a4ae7fe481bc6e97acde1e


---

> +#define LOG_BUF_LEN_MAX (u32)(1 << 31)
[..]
> +	if (size > (u64)LOG_BUF_LEN_MAX) {
> +		size = (u64)LOG_BUF_LEN_MAX;
> +		pr_err("log_buf over 2G is not supported.\n");
> +	}

Why not INT_MAX?


> +	pr_info("log_buf_len: %u bytes\n", log_buf_len);
> +	pr_info("early log buf free: %u(%u%%)\n",
>  		free, (free * 100) / __LOG_BUF_LEN);

Can 'free * 100' overflow?

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-09 13:57         ` Sergey Senozhatsky
@ 2018-10-10  8:09           ` Petr Mladek
  2018-10-10  8:21             ` Sergey Senozhatsky
  0 siblings, 1 reply; 18+ messages in thread
From: Petr Mladek @ 2018-10-10  8:09 UTC (permalink / raw)
  To: Sergey Senozhatsky; +Cc: zhe.he, rostedt, linux-kernel

On Tue 2018-10-09 22:57:58, Sergey Senozhatsky wrote:
> On (10/09/18 15:05), Petr Mladek wrote:
> > > 
> > > Yeah, I think we gonna have problems even with a 4G logbuf and a 32-bit
> > > user-space doing syslog(int len).
> > > 
> > > I agree on the "not motivated enough" part ;)
> > 
> > OK, I have pushed an updated patch that has the limit 2GB
> > into printk.git, for-4.20 branch.
> > 
> > Note that it is slightly different than the yesterday's proposal.
> > I made a mistake in testing and still compared with UNIT_MAX.
> > 
> > The pushed version can be seen at
> > https://git.kernel.org/pub/scm/linux/kernel/git/pmladek/printk.git/commit/?h=for-4.20&id=e6fe3e5b7d16e8f146a4ae7fe481bc6e97acde1e
> 
> 
> ---
> 
> > +#define LOG_BUF_LEN_MAX (u32)(1 << 31)
> [..]
> > +	if (size > (u64)LOG_BUF_LEN_MAX) {
> > +		size = (u64)LOG_BUF_LEN_MAX;
> > +		pr_err("log_buf over 2G is not supported.\n");
> > +	}
> 
> Why not INT_MAX?

INT_MAX is 0x7fffffff but we need 0x80000000. I did not find
any predefined macro.

> 
> > +	pr_info("log_buf_len: %u bytes\n", log_buf_len);
> > +	pr_info("early log buf free: %u(%u%%)\n",
> >  		free, (free * 100) / __LOG_BUF_LEN);
> 
> Can 'free * 100' overflow?

Good question. It uses the size of the static buffer. If I count
correctly then we are on the safe side because LOG_BUF_SHIFT
is limited by

	range 12 25

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G
  2018-10-10  8:09           ` Petr Mladek
@ 2018-10-10  8:21             ` Sergey Senozhatsky
  0 siblings, 0 replies; 18+ messages in thread
From: Sergey Senozhatsky @ 2018-10-10  8:21 UTC (permalink / raw)
  To: Petr Mladek; +Cc: Sergey Senozhatsky, zhe.he, rostedt, linux-kernel

On (10/10/18 10:09), Petr Mladek wrote:
> > > +#define LOG_BUF_LEN_MAX (u32)(1 << 31)
> > [..]
> > > +	if (size > (u64)LOG_BUF_LEN_MAX) {
> > > +		size = (u64)LOG_BUF_LEN_MAX;
> > > +		pr_err("log_buf over 2G is not supported.\n");
> > > +	}
> > 
> > Why not INT_MAX?
> 
> INT_MAX is 0x7fffffff but we need 0x80000000. I did not find
> any predefined macro.

Hmm, OK. I need to think about it more.

> > > +	pr_info("log_buf_len: %u bytes\n", log_buf_len);
> > > +	pr_info("early log buf free: %u(%u%%)\n",
> > >  		free, (free * 100) / __LOG_BUF_LEN);
> > 
> > Can 'free * 100' overflow?
> 
> Good question. It uses the size of the static buffer. If I count
> correctly then we are on the safe side because LOG_BUF_SHIFT
> is limited by
> 
> 	range 12 25

I didn't know there was Kconfig "range" :)

$ echo $(((1<<25)*100))
3355443200

	-ss

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-10-10  8:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-29 16:45 [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line zhe.he
2018-09-29 16:45 ` [PATCH v5 2/4] printk: Correct wrong casting zhe.he
2018-10-02  5:50   ` Sergey Senozhatsky
2018-10-08 13:03     ` Petr Mladek
2018-09-29 16:45 ` [PATCH v5 3/4] printk: Add KBUILD_MODNAME and remove a redundant print prefix zhe.he
2018-10-02  5:52   ` Sergey Senozhatsky
2018-10-08 13:04   ` Petr Mladek
2018-09-29 16:45 ` [PATCH v5 4/4] printk: Give error on attempt to set log buffer length to over 4G zhe.he
2018-10-02  5:51   ` Sergey Senozhatsky
2018-10-08 13:59   ` Petr Mladek
2018-10-08 14:59     ` Sergey Senozhatsky
2018-10-09 13:05       ` Petr Mladek
2018-10-09 13:57         ` Sergey Senozhatsky
2018-10-10  8:09           ` Petr Mladek
2018-10-10  8:21             ` Sergey Senozhatsky
2018-10-08 16:30     ` He Zhe
2018-10-02  5:48 ` [PATCH v5 1/4] printk: Fix panic caused by passing log_buf_len to command line Sergey Senozhatsky
2018-10-08 13:01 ` Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).