Bug 631924

Created attachment 446043 [details]
abort directory

This was found during Bug 631564 analysis.  That bug contains all information I have available.  Corey is contact on this bug.

looking through the stack trace this appears to be a problem with the console handling code added for bug 615912

ugh, there are a few logic errors here that mean this may crash depending on where in the command line the console= line is.

So I've been investigating this issue tonight, trying to figure out the ramifications of this bug.

The backstory is in early August I attempted to fix a bug with how plymouth handled duplicate console entries on the kernel command line. An example of a problematic console line would be:

  kernel ... console=ttyS0 console=tty0 rhgb quiet console=ttyS0

In the above line ttyS0 shows up twice.  Plymouth wouldn't weed out the dupes before, so the user would see everything twice (or 3 times or however many dupliates they had).  Besides the cosmetic issue of duplicate boot messages, it would also cause terminal settings to get reset twice, and cause reads from the tty to happen twice.  These bugs meant if a user accidentally had duplicate console entries, they would not be able to login on the console that was specified more than once on the kernel command line.

My initial fix for this issue was wrong.  While it seemingly fixed the original bug, it also broke bootup for a subset of users (see bug 615912 for more of the gritty details).

To my horror, it's clear that my second fix is wrong, too.  It has some blatant logic errors in it which I alluded to in comment 5.  These errors are because some lines of code were changed to store the console in a temporary variable, but some corresponding lines of code weren't changed to read from that temporary variable.

I didn't notice this after writing the patch when I reread through it before committing it because the lines that needed to be changed were outside the context of the patch.  I made the mistake in the first place, presumably, because I was trying to get a fix out for my first mistake too quickly.

The upshot is that plymouthd crashes if you have console= entries on your kernel command line.  I was wrong in comment 5 to say it was dependent on where in the kernel command line the entries showed up.  It happens if there are any console entries anywhere on the line.

It's not as dire as it may seem at first, though.  plymouthd has 3 main functions:

1) provide a boot splash during boot up
2) Ask the user for their password if they have encrypted disks
3) log boot messages to /var/log/boot.log for later review

1) isn't relevant when the user has serial consoles.  We forcefully hide the boot splash in that case anyway. 

2) is extremely important if users have encrypted disks.  Fortunately, if the plymouth daemon isn't available the plymouth client will shoulder the responsibilities of asking for passwords. It does this to handle cases where the user adds init=/bin/sh to the command line.  That fallback code should mitigate most of the problems caused by this bug.

3) is unfortunate, but fairly low severity. boot.log is a new feature in RHEL 6 and it's primarily useful for non-serial console users who have the boot splash occluding boot messages during boot up.

1) and 2) together make the crash almost transparent.  That's probably why no one noticed this issue for a month even though serial consoles are widely used with by QE.

We're probably going to need to do a plymouth update in 6.1 for bug 625209 .  I suggest we roll the fix for this at the same time.

I am going to do a fresh install with encrypted disks to make sure the client fallback in 2) is working properly, though.

Just to follow up, I added an encrypted /opt partition to my system and rebooted.  The fallback code worked fine.

Two side effects:

1) The prompt text is different:

Enter passphrase for /dev/sdb1:

instead of the slightly nicer

/opt is password protected:

2) When the user types their password, they don't see bullets, it's just blank as they type

These are both side effects of the fallback code piping the user directly to the  cryptsetup command without plymouthd being in the picture.

Also, I want to add that I do have a patch that should fix this once and for all.

Created attachment 446134 [details]
fix logic errors

This is the patch mentioned in the previous comment.

devack+

This is a pretty important one to get in 6.1 I think.

These fixes are now in plymouth-0.8.3-18.el6

    Technical note added. If any revisions are required, please edit the "Technical Notes" field
    accordingly. All revisions will be proofread by the Engineering Content Services team.
    
    New Contents:
Previously, the plymouth daemon terminated unexpectedly if there were any "console=" entries on the kernel command line. As a consequence, boot messages were not properly logged. With this update, this bug has been fixed, the plymouth daemon now handles console entries on the kernel command line without any crashes and boot messages are written to the log file as expected.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0686.html