Re: klogd dies, possible emit_log_char() vs do_syslog(..READ..) race.

[Peter Chubb <peter.chubb@nicta.com.au>]

>>>>> "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