Messages in this thread | | | Date | Wed, 27 Jul 2011 12:11:05 +1000 | From | Peter Chubb <> | Subject | Re: klogd dies, possible emit_log_char() vs do_syslog(..READ..) race. |
| |
>>>>> "Peter" == Peter Zijlstra <peterz@infradead.org> writes:
Peter> On Tue, 2011-07-26 at 11:08 +1000, Peter Chubb wrote: >> Hi, >> >> Since mid May, my klogd has been turning into a 100% CPU hog at >> regular intervals. Turns out that sometimes reads from /proc/kmsg >> return with zero bytes read --- pointing to a race between >> do_syslog(SYSLOG_ACTION_READ...) and emit_log_char(). (In Debian, >> klogd is invoked with a dd frontend, as dd if=/proc/kmsg >> of=/var/run/klogd bs=1 where /var/run/klogd is a named pipe; this >> dd dies when it reads zero bytes).
Peter> Well I suppose that's a bug anyway since if someone managed to Peter> have multiple consumers of the data you'll get into that same Peter> situation, therefore using dd might not be the sanest thing to Peter> do.
Indeed -- and this *is* the case on my system (somehow, rsyslogd and klogd were both installed).
Peter> Suppose someone does cat /proc/kmsg > /dev/null, just for kicks Peter> ;-)
They'd have to be privileged in some way to do that.
Peter> Well there is a distinct lack of serialization between the Peter> log_start, log_end usage in wait_event_interruptible() and Peter> emit_log_char().
Peter> From what I can see one has to hold the logbuf_lock to get a Peter> stable reading, so if the code in SYSLOG_ACTION_READ requires Peter> this for proper functioning its buggy.
You don't need a stable reading -- you just need to be assured that there's something there to read. And if there are multiple readers all bets are off -- in the (usual) single reader case, the count of things in the buffer should only increase, not decrease, during the race.
Peter> That said, this is the very first time I ever looked at that Peter> code ;-)
Yes there's no maintainer listed for kernel/printk.c --- I emailed you because you were the last to touch something in that file, so I thought that the code should be in your head at least a little bit :-)
Peter> Does something like the (completely untested) patch below also Peter> cure things?
Nope. There's still room for something to happen between the wakeup and the test. I'm beginning to think my ugly band-aid is the correct patch!
-- Dr Peter Chubb http://www.gelato.unsw.edu.au peterc AT gelato.unsw.edu.au http://www.ertos.nicta.com.au ERTOS within National ICT Australia
| |