linux-kernel - Re: klogd dies, possible emit_log_char() vs do

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <w4aac0eame.wl%peter@chubb.wattle.id.au>
Date:	Wed, 27 Jul 2011 12:11:05 +1000
From:	Peter Chubb <peter.chubb@...ta.com.au>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Peter Chubb <peter.chubb@...ta.com.au>, akpm@...ux-foundation.org,
	torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: klogd dies, possible emit_log_char() vs do_syslog(..READ..) race.

>>>>> "Peter" == Peter Zijlstra <peterz@...radead.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/