Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-04 Thread Sergey Senozhatsky
On (12/04/18 19:06), Tetsuo Handa wrote: > On 2018/12/04 11:56, Sergey Senozhatsky wrote: > > Can we please have something better than 'f'? > > OK. To pass 80 column limit, one line increased. ;-) Ah, so that was the reason :) [..] > To close this race, get a snapshot value of printk_time and

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-04 Thread Sergey Senozhatsky
On (12/04/18 19:06), Tetsuo Handa wrote: > On 2018/12/04 11:56, Sergey Senozhatsky wrote: > > Can we please have something better than 'f'? > > OK. To pass 80 column limit, one line increased. ;-) Ah, so that was the reason :) [..] > To close this race, get a snapshot value of printk_time and

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-04 Thread Tetsuo Handa
On 2018/12/04 11:56, Sergey Senozhatsky wrote: > Can we please have something better than 'f'? OK. To pass 80 column limit, one line increased. ;-) < + bool f = syslog_partial ? syslog_time : printk_time; > + bool time = syslog_partial ? syslog_time :

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-04 Thread Tetsuo Handa
On 2018/12/04 11:56, Sergey Senozhatsky wrote: > Can we please have something better than 'f'? OK. To pass 80 column limit, one line increased. ;-) < + bool f = syslog_partial ? syslog_time : printk_time; > + bool time = syslog_partial ? syslog_time :

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Sergey Senozhatsky
On (12/02/18 14:02), Tetsuo Handa wrote: > > @@ -1541,11 +1545,13 @@ int do_syslog(int type, char __user *buf, int len, > int source) > } else { > u64 seq = syslog_seq; > u32 idx = syslog_idx; > + bool f =

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Sergey Senozhatsky
On (12/02/18 14:02), Tetsuo Handa wrote: > > @@ -1541,11 +1545,13 @@ int do_syslog(int type, char __user *buf, int len, > int source) > } else { > u64 seq = syslog_seq; > u32 idx = syslog_idx; > + bool f =

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Tetsuo Handa
On 2018/12/03 23:14, Petr Mladek wrote: > Well, IMHO, it does not explain the pagefault above. The copy_to_user() > could come either from syslog_print() or from syslog_print_all(). They > both have their own checks that prevent the buf overflow. > > The code is tricky but it looks safe now. Is

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Tetsuo Handa
On 2018/12/03 23:14, Petr Mladek wrote: > Well, IMHO, it does not explain the pagefault above. The copy_to_user() > could come either from syslog_print() or from syslog_print_all(). They > both have their own checks that prevent the buf overflow. > > The code is tricky but it looks safe now. Is

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Petr Mladek
On Fri 2018-11-30 23:33:39, Tetsuo Handa wrote: > and (I can't find the reproducer but) sometimes crashes like > > [ 451.988242] BUG: pagefault on kernel address 0x8881337fa000 in > non-whitelisted uaccess > [ 451.990465] BUG: unable to handle kernel paging request at 8881337fa000 > [

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Petr Mladek
On Fri 2018-11-30 23:33:39, Tetsuo Handa wrote: > and (I can't find the reproducer but) sometimes crashes like > > [ 451.988242] BUG: pagefault on kernel address 0x8881337fa000 in > non-whitelisted uaccess > [ 451.990465] BUG: unable to handle kernel paging request at 8881337fa000 > [

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Petr Mladek
On Sun 2018-12-02 14:02:28, Tetsuo Handa wrote: > On 2018/12/02 8:49, kbuild test robot wrote: > >> kernel/printk/printk.c:2396:5: error: 'printk_time' undeclared (first use > >> in this function) > ^~~ > Thanks. printk_time depends on CONFIG_PRINTK=y. Added a dummy definition. >

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-03 Thread Petr Mladek
On Sun 2018-12-02 14:02:28, Tetsuo Handa wrote: > On 2018/12/02 8:49, kbuild test robot wrote: > >> kernel/printk/printk.c:2396:5: error: 'printk_time' undeclared (first use > >> in this function) > ^~~ > Thanks. printk_time depends on CONFIG_PRINTK=y. Added a dummy definition. >

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-01 Thread Tetsuo Handa
On 2018/12/02 8:49, kbuild test robot wrote: >> kernel/printk/printk.c:2396:5: error: 'printk_time' undeclared (first use in >> this function) ^~~ Thanks. printk_time depends on CONFIG_PRINTK=y. Added a dummy definition. >From f903b9fa36159472a207f93e2405e45e3999f650 Mon Sep 17

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-01 Thread Tetsuo Handa
On 2018/12/02 8:49, kbuild test robot wrote: >> kernel/printk/printk.c:2396:5: error: 'printk_time' undeclared (first use in >> this function) ^~~ Thanks. printk_time depends on CONFIG_PRINTK=y. Added a dummy definition. >From f903b9fa36159472a207f93e2405e45e3999f650 Mon Sep 17

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-01 Thread Tetsuo Handa
>From bc9ea8e7878a618fd4490ec287eb691fee144acb Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sat, 1 Dec 2018 19:37:42 +0900 Subject: [PATCH] printk: fix printk_time race. Since printk_time can be toggled via /sys/module/printk/parameters/time , it is not safe to assume that output length

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-12-01 Thread Tetsuo Handa
>From bc9ea8e7878a618fd4490ec287eb691fee144acb Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Sat, 1 Dec 2018 19:37:42 +0900 Subject: [PATCH] printk: fix printk_time race. Since printk_time can be toggled via /sys/module/printk/parameters/time , it is not safe to assume that output length

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-30 Thread Tetsuo Handa
On 2018/11/30 21:30, Petr Mladek wrote: > I am not fully happy with the solution passing time parameter. > It is less secure. But it would not break compatibility. > This particular race might be solved the following way: > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-30 Thread Tetsuo Handa
On 2018/11/30 21:30, Petr Mladek wrote: > I am not fully happy with the solution passing time parameter. > It is less secure. But it would not break compatibility. > This particular race might be solved the following way: > > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-30 Thread Petr Mladek
On Fri 2018-11-30 11:26:02, Tetsuo Handa wrote: > Petr Mladek wrote: > > See syslog_print_all() and kmsg_dump_get_buffer(). They > > start with calling msg_print_text() many times to calculate > > how many messages fit into the given buffer. Then they > > blindly copy the messages into the buffer.

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-30 Thread Petr Mladek
On Fri 2018-11-30 11:26:02, Tetsuo Handa wrote: > Petr Mladek wrote: > > See syslog_print_all() and kmsg_dump_get_buffer(). They > > start with calling msg_print_text() many times to calculate > > how many messages fit into the given buffer. Then they > > blindly copy the messages into the buffer.

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Sergey Senozhatsky
On (11/29/18 15:19), Petr Mladek wrote: > This fixes a real problem. It might even avoid a crash. > > See syslog_print_all() and kmsg_dump_get_buffer(). They > start with calling msg_print_text() many times to calculate > how many messages fit into the given buffer. Then they > blindly copy the

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Sergey Senozhatsky
On (11/29/18 15:19), Petr Mladek wrote: > This fixes a real problem. It might even avoid a crash. > > See syslog_print_all() and kmsg_dump_get_buffer(). They > start with calling msg_print_text() many times to calculate > how many messages fit into the given buffer. Then they > blindly copy the

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Tetsuo Handa
Petr Mladek wrote: > See syslog_print_all() and kmsg_dump_get_buffer(). They > start with calling msg_print_text() many times to calculate > how many messages fit into the given buffer. Then they > blindly copy the messages into the buffer. > > Now, the buffer might overflow if the size is

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Tetsuo Handa
Petr Mladek wrote: > See syslog_print_all() and kmsg_dump_get_buffer(). They > start with calling msg_print_text() many times to calculate > how many messages fit into the given buffer. Then they > blindly copy the messages into the buffer. > > Now, the buffer might overflow if the size is

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Petr Mladek
On Sat 2018-11-24 19:50:13, Tetsuo Handa wrote: > Since /sys/module/printk/parameters/time can change from N to Y between > "msg_print_text() called print_prefix() with buf == NULL" and > "msg_print_text() again calls print_prefix() with buf != NULL", it is not > safe for print_time() to

Re: [PATCH] printk: don't unconditionally shortcut print_time()

2018-11-29 Thread Petr Mladek
On Sat 2018-11-24 19:50:13, Tetsuo Handa wrote: > Since /sys/module/printk/parameters/time can change from N to Y between > "msg_print_text() called print_prefix() with buf == NULL" and > "msg_print_text() again calls print_prefix() with buf != NULL", it is not > safe for print_time() to

[PATCH] printk: don't unconditionally shortcut print_time()

2018-11-24 Thread Tetsuo Handa
Since /sys/module/printk/parameters/time can change from N to Y between "msg_print_text() called print_prefix() with buf == NULL" and "msg_print_text() again calls print_prefix() with buf != NULL", it is not safe for print_time() to unconditionally return 0 if printk_time == false. Signed-off-by:

[PATCH] printk: don't unconditionally shortcut print_time()

2018-11-24 Thread Tetsuo Handa
Since /sys/module/printk/parameters/time can change from N to Y between "msg_print_text() called print_prefix() with buf == NULL" and "msg_print_text() again calls print_prefix() with buf != NULL", it is not safe for print_time() to unconditionally return 0 if printk_time == false. Signed-off-by: