| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 6 Mar 2016 22:27:03 +0900
From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc: sergey.senozhatsky.work@...il.com, akpm@...ux-foundation.org,
jack@...e.com, pmladek@...e.com, tj@...nel.org,
linux-kernel@...r.kernel.org, sergey.senozhatsky@...il.com,
jack@...e.cz
Subject: Re: [RFC][PATCH v2 1/2] printk: Make printk() completely async
On (03/06/16 20:06), Tetsuo Handa wrote:
[..]
> > do you mean a new worker allocation delay and a MAYDAY timer delay?
> >
>
> I don't know what MAYDAY is. I'm talking about a situation where printing_work
> work item is not processed (i.e. printing_work_func() is not called) until
> current work item calls schedule_timeout_*().
>
> We had a problem that since vmstat_work work item was using system_wq,
> vmstat_work work item was not processed (i.e. vmstat_update() was not called) if
> kworker was looping inside memory allocator without calling schedule_timeout_*()
> due to disk_events_workfn() doing GFP_NOIO allocation).
hm, just for note, none of system-wide wqs seem to have a ->rescuer thread
(WQ_MEM_RECLAIM).
[..]
> Even if you use printk_wq with WQ_MEM_RECLAIM for printing_work work item,
> printing_work_func() will not be called until current work item calls
> schedule_timeout_*(). That will be an undesirable random delay. If you use
> a dedicated kernel thread rather than a dedicated workqueue with WQ_MEM_RECLAIM,
> we can avoid this random delay.
hm. yes, seems that it may take some time until workqueue wakeup() a ->rescuer thread.
need to look more.
[..]
> > console_lock();
> > for (...) {
> > do_foo() {
> > ...
> > pr_err(" ... foo message ...\n");
> > ...
> > }
> > }
> > console_unlock();
> >
> > then yes, nothing will be printed until that process executes console_unlock(),
> > because it's console_unlock() that pushes the messages to console drivers.
>
> Yes, I meant a sequence like
>
> console_lock();
> ptr = kmalloc(GFP_KERNEL);
> kfree(ptr);
> console_unlock();
>
> and kmalloc() prints OOM killer messages rather than failing that allocation.
> Are we sure that there is no such usage?
such usage is quite possible.
problems that I have with console_lock()/console_unlock() is that
these functions serve a double purpose: exclusive printk() lock and a
console_drivers list lock.
**** I haven't really thought about it yet, but I want to split it. ****
console_lock()/console_unlock() can be executed by user space processes
(inside system calls). for example:
SyS_open()
...
chrdev_open()
tty_open()
console_device()
console_lock()
console_unlock()
for (;;) {
call_console_drivers()
}
or doing `cat /proc/consoles`
SyS_read()
vfs_read()
proc_reg_read()
seq_read()
c_stop()
console_unlock()
for (;;) {
call_console_drivers()
}
which can introduce two nasty problems:
1) console_lock() may put user space process in TASK_UNINTERRUPTIBLE for
unknown period of time -- until current console_sem owner will not
finish print loop in console_unlock(). no signals, heartbeats, etc.
will be processed by this user space process.
2) user space process may have to spend an unknown period of time in
console_unlock() later, pushing "3rd party" kernel messages to
console drivers. again, not really good.
(kthreads can suffer here too, sure).
in the examples above, a process just wanted to iterate the console_drivers
list in read access mode. so, for instance, in
struct tty_driver *console_device(int *index)
{
struct console *c;
struct tty_driver *driver = NULL;
console_lock();
for_each_console(c) {
if (!c->device)
continue;
driver = c->device(c, index);
if (driver)
break;
}
console_unlock();
return driver;
}
instead of console_lock()/console_unlock()->call_console_drivers()
it could do (very schematically)
read_lock_console();
for_each_console(c) {
if (!c->device)
continue;
driver = c->device(c, index);
if (driver)
break;
}
read_unlock_console();
and in functions that modify the list, the lock can be acquired in
write mode. example,
int unregister_console(struct console *console)
{
write_lock_console();
...
for (a=console_drivers->next, b=console_drivers ;
a; b=a, a=b->next) {
if (a == console) {
b->next = a->next;
res = 0;
break;
}
}
}
...
if (console_drivers != NULL && console->flags & CON_CONSDEV)
console_drivers->flags |= CON_CONSDEV;
console->flags &= ~CON_ENABLED;
write_unlock_unlock();
printk(), thus, will take its own "another" exclusive lock, to guarantee
that only one process can call_console_drivers(), and it will take the
console_drivers list lock in read mode. so other process(-es) that also
want to access console_drivers list in read mode will not wait in
TASK_UNINTERRUPTIBLE.
of course, this means that console_unlock() now does not print
anything to console drivers. it's printk() duty to do this.
and the last statement can be very hard to sell; because I don't know
for sure if there is a code in the kernel that depends on the fact that
console_lock() stops printk() and that console_unlock() prints all of
the printk messages.
that "lock split" also can fix another *theoretical* scenario: suppose,
that for some reason someone's setup has both a) huge number of printk()
calls and b) a relatively huge number of console_lock()/console_unlock()
calls, all happening simultaneously. while async printk helps in (a),
case (b) "detours" the printk async code; and if cpus at lest some of the
cpus that do console_lock()/console_unlock() also disable IRQs or
preemption, then lockups and all the bad things are very much likely.
just an idea.
-ss
Powered by blists - more mailing lists