Bug 80553 - plymouthd exists prematurely so boot splash doesn't work with 0.9.0
Summary: plymouthd exists prematurely so boot splash doesn't work with 0.9.0
Status: RESOLVED MOVED
Alias: None
Product: plymouth
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Ray Strode [halfline]
QA Contact:
URL:
Whiteboard:
Keywords:
: 87993 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-06-26 09:26 UTC by Lukas Anzinger
Modified: 2018-08-07 09:25 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments
plymouth debug log file for 0.9.0 (unpatched) (4.71 KB, text/plain)
2014-06-26 09:26 UTC, Lukas Anzinger
Details
plymouth debug log file for 0.9.0 (check for seat->terminal != NULL) (72.11 KB, text/plain)
2014-06-26 17:14 UTC, Lukas Anzinger
Details
plymouth debug log file for 0.9.0 (check for seat->terminal != NULL and udev seat always true) (72.62 KB, text/plain)
2014-06-26 17:29 UTC, Lukas Anzinger
Details

Description Lukas Anzinger 2014-06-26 09:26:28 UTC
Created attachment 101794 [details]
plymouth debug log file for 0.9.0 (unpatched)

Hi,

I recently upgraded to plymouth 0.9.0 on Debian Sid and plymouth stopped working.

I'm using the following setup:

- Linux 3.14.7 (linux-image-3.14-1-686-pae 3.14.7-1)
- plymouth 0.9.0 (plymouth 0.9.0-3)
- uvesafb with AMD Radeon E6760 Embedded (uvesafb mode_option=1280x800-32 scroll=ywrap)

$ cat /etc/plymouth/plymouthd.conf
# Administrator customizations go in this file
[Daemon]
Theme=fade-in
ShowDelay=1

It's a clean installation with nothing more than a basic Debian Sid and plymouth on top. plymouth 0.8.8 worked but 0.9.0 doesn't. I've attached the log file that maybe could shed some light on the matter. It's rather short so I believe plymouthd exits prematurely.

I bisected the problem and I believe the following commit causes a regression on my setup: 

commit 7e594f72550d5eeb02507ade602d7f95391e77ac
Author: Ray Strode <rstrode@redhat.com>
Date:   Thu Mar 6 14:42:16 2014 -0500

    seat: make sure to open terminal when adding text displays
    
    If we have a pixel display, the renderer will handle opening the
    associated terminal. but if we don't have a pixel display, something
    needs to open the terminal.
    
    This commit adds code to do that.

diff --git a/src/libply-splash-core/ply-seat.c b/src/libply-splash-core/ply-seat.c
index 541b29e..2ac8bf7 100644
--- a/src/libply-splash-core/ply-seat.c
+++ b/src/libply-splash-core/ply-seat.c
@@ -102,6 +102,19 @@ add_text_displays (ply_seat_t *seat)
 {
   ply_text_display_t *display;
 
+  if (!ply_terminal_is_open (seat->terminal))
+    {
+      if (!ply_terminal_open (seat->terminal))
+        {
+          ply_trace ("could not add terminal %s: %m",
+                     ply_terminal_get_name (seat->terminal));
+          return;
+        }
+    }
+
+  ply_trace ("adding text display for terminal %s",
+             ply_terminal_get_name (seat->terminal));
+
   display = ply_text_display_new (seat->terminal);
   ply_list_append_data (seat->text_displays, display);
 }

If I revert that commit on top of 0.9.0 plymouth works again.

Regards,

Lukas
Comment 1 Ray Strode [halfline] 2014-06-26 12:31:31 UTC
if you wrap the entire added lines in

if (seat->terminal != NULL) {
  .... all the lines here ...
}

does it also continue to work?
Comment 2 Lukas Anzinger 2014-06-26 16:29:55 UTC
Hi,

yes, it still works. I could reproduce the problem on a different hardware (Intel 855GM, i915) but only when using uvesafb (nomodeset on cmdline and uvesafb configuration in /etc/initramfs/modules).

diff --git a/src/libply-splash-core/ply-seat.c b/src/libply-splash-core/ply-seat.c
index 2ac8bf7..0f533d5 100644
--- a/src/libply-splash-core/ply-seat.c
+++ b/src/libply-splash-core/ply-seat.c
@@ -102,6 +102,8 @@ add_text_displays (ply_seat_t *seat)
 {
   ply_text_display_t *display;
 
+  if (seat->terminal != NULL)
+    {
   if (!ply_terminal_is_open (seat->terminal))
     {
       if (!ply_terminal_open (seat->terminal))
@@ -115,6 +117,12 @@ add_text_displays (ply_seat_t *seat)
   ply_trace ("adding text display for terminal %s",
              ply_terminal_get_name (seat->terminal));
 
+    }
+  else
+    {
+      ply_trace("seat->terminal == NULL");
+    }
+
   display = ply_text_display_new (seat->terminal);
   ply_list_append_data (seat->text_displays, display);
 }

From the log file:

[ply-seat.c:123]                             add_text_displays:seat->terminal == NULL

So seat->terminal is NULL which isn't checked in ply_terminal_is_open() and so plymouthd dereferences a NULL pointer.

Regards,

Lukas
Comment 3 Lukas Anzinger 2014-06-26 17:13:12 UTC
So the question is why seat->terminal can be NULL.

A seat is created in create_seats_from_udev() which calls create_seats_for_subsystem().

Digging through the log file ...

[ply-device-manager.c:705]                        create_seats_from_udev:Looking for devices from udev
[ply-device-manager.c:284]                    create_seats_for_subsystem:creating seats for drm devices
[ply-device-manager.c:284]                    create_seats_for_subsystem:creating seats for frame buffer devices
[ply-device-manager.c:303]                    create_seats_for_subsystem:found device /sys/devices/platform/uvesafb.0/graphics/fb0
[ply-device-manager.c:311]                    create_seats_for_subsystem:device is initialized
[ply-device-manager.c:329]                    create_seats_for_subsystem:device doesn't have a seat tag
[ply-device-manager.c:303]                    create_seats_for_subsystem:found device /sys/devices/virtual/graphics/fbcon
[ply-device-manager.c:334]                    create_seats_for_subsystem:it's not initialized

... we can see that a frame buffer device is found and device is initialized but it doesn't have a seat tag so create_seats_for_subsystem() ignores it (the latest patch in plymouth 0.9.0-3 kind of addresses that problem, see also https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=751726 but in this build the patch is not applied).

Then non-graphical seat for /dev/tty7 is created.

[ply-device-manager.c:713]                        create_seats_from_udev:Creating non-graphical seat, since there's no suitable graphics hardware
[ply-device-manager.c:641]    create_seat_for_terminal_and_renderer_type:creating seat for /dev/tty7 (renderer type: 4294967295) (terminal: /dev/tty7)

[...]

And after some time (in run level S and not in the initramfs) a seat for /dev/fb0 with no terminal attached (-> seat->terminal == NULL).

[ply-device-manager.c:357]                                 on_udev_event:got add event for device fb0
[ply-device-manager.c:178]                   create_seat_for_udev_device:device is for local console: no
[ply-device-manager.c:191]                   create_seat_for_udev_device:device subsystem is graphics
[ply-device-manager.c:200]                   create_seat_for_udev_device:found frame buffer device /dev/fb0
[ply-device-manager.c:135]                      fb_device_has_drm_device:trying to find associated drm node for fb device (path: platform-uvesafb.0)
[ply-device-manager.c:161]                      fb_device_has_drm_device:no card entry!
[ply-device-manager.c:641]    create_seat_for_terminal_and_renderer_type:creating seat for /dev/fb0 (renderer type: 2) (terminal: none)
[ply-renderer.c:234]                      ply_renderer_open_plugin:trying to open renderer plugin /usr/lib/i386-linux-gnu/plymouth/renderers/frame-buffer.so
[./plugin.c:259]                                create_backend:creating renderer backend for device /dev/fb0
[./plugin.c:515]                                  query_device:32 bpp (8, 8, 8, 8) with rowstride 3200
[./plugin.c:275]                               initialize_head:initializing 800x600 head
[ply-renderer.c:256]                      ply_renderer_open_plugin:opened renderer plugin /usr/lib/i386-linux-gnu/plymouth/renderers/frame-buffer.so
[ply-seat.c:80]                            add_pixel_displays:Adding displays for 1 heads
[ply-seat.c:123]                             add_text_displays:seat->terminal == NULL

Regards,

Lukas
Comment 4 Lukas Anzinger 2014-06-26 17:14:25 UTC
Created attachment 101817 [details]
plymouth debug log file for 0.9.0 (check for seat->terminal != NULL)
Comment 5 Lukas Anzinger 2014-06-26 17:28:57 UTC
Hi,

I've also tried plymouth with seat->terminal != NULL and the seat patch from Debian:

diff --git a/src/libply-splash-core/ply-device-manager.c b/src/libply-splash-core/ply-device-manager.c
index dbc203d..e9f8519 100644
--- a/src/libply-splash-core/ply-device-manager.c
+++ b/src/libply-splash-core/ply-device-manager.c
@@ -313,7 +313,7 @@ create_seats_for_subsystem (ply_device_manager_t *manager,
           /* We only care about devices assigned to a (any) seat. Floating
            * devices should be ignored.
            */
-          if (udev_device_has_tag (device, "seat"))
+          if (true)
             {
               const char *node;
               node = udev_device_get_devnode (device);

Now a seat is immediately created for /dev/fb0 because a frame buffer device is found but the terminal for that device is still NULL.

[ply-device-manager.c:705]                        create_seats_from_udev:Looking for devices from udev
[ply-device-manager.c:284]                    create_seats_for_subsystem:creating seats for drm devices
[ply-device-manager.c:284]                    create_seats_for_subsystem:creating seats for frame buffer devices
[ply-device-manager.c:303]                    create_seats_for_subsystem:found device /sys/devices/platform/uvesafb.0/graphics/fb0
[ply-device-manager.c:311]                    create_seats_for_subsystem:device is initialized
[ply-device-manager.c:322]                    create_seats_for_subsystem:found node /dev/fb0
[ply-device-manager.c:178]                   create_seat_for_udev_device:device is for local console: no
[ply-device-manager.c:191]                   create_seat_for_udev_device:device subsystem is graphics
[ply-device-manager.c:200]                   create_seat_for_udev_device:found frame buffer device /dev/fb0
[ply-device-manager.c:135]                      fb_device_has_drm_device:trying to find associated drm node for fb device (path: (null))
[ply-device-manager.c:161]                      fb_device_has_drm_device:no card entry!
[ply-device-manager.c:641]    create_seat_for_terminal_and_renderer_type:creating seat for /dev/fb0 (renderer type: 2) (terminal: none)
[ply-renderer.c:234]                      ply_renderer_open_plugin:trying to open renderer plugin /usr/lib/i386-linux-gnu/plymouth/renderers/frame-buffer.so
[./plugin.c:259]                                create_backend:creating renderer backend for device /dev/fb0
[./plugin.c:515]                                  query_device:32 bpp (8, 8, 8, 8) with rowstride 3200
[./plugin.c:275]                               initialize_head:initializing 800x600 head
[ply-renderer.c:256]                      ply_renderer_open_plugin:opened renderer plugin /usr/lib/i386-linux-gnu/plymouth/renderers/frame-buffer.so
[ply-seat.c:80]                            add_pixel_displays:Adding displays for 1 heads
[ply-seat.c:123]                             add_text_displays:seat->terminal == NULL

Regards,

Lukas
Comment 6 Lukas Anzinger 2014-06-26 17:29:19 UTC
Created attachment 101818 [details]
plymouth debug log file for 0.9.0 (check for seat->terminal != NULL and udev seat always true)
Comment 7 Lukas Anzinger 2014-07-02 09:48:52 UTC
Is there anything I can do to help you resolving that issue? I could also send you hardware and a preinstalled Debian.
Comment 8 Ray Strode [halfline] 2014-07-14 12:28:22 UTC
Hey, sorry for the slow reply with this.  So I think what's probably happening is device_is_for_local_console() in ply-device-manager.c is probably failing, so create_seat_for_udev_device() is creating a seat with a NULL terminal, and then we're failing to handle a NULL terminal seat very well.

I've pushed this commit to master:

http://cgit.freedesktop.org/plymouth/commit/?id=84eb4381db85877a9a56b35994e6c10d43e46ebe

to try to handle the NULL terminal case better.

There's more to the story though.  device_is_for_local_console() is a function that checks if the video device is the "boot_vga" device, the one that was active at boot time.  I think the idea in my mind was to avoid opening a terminal more than once if there were multiple video cards associated with the machine.  This idea has two failings:

1) There may be a case where no video device has boot_vga, I guess, in which case a terminal won't ever get opened and so password input won't work
2) if conceptually, a seat is a "monitor and a keyboard" then the abstraction is just wrong.

So I need to rework this a bit...
Comment 9 Leho Kraav (:macmaN :lkraav) 2014-08-02 19:05:54 UTC
We are looking at this at https://bugs.gentoo.org/show_bug.cgi?id=517572 and I pulled git master yesterday. Still doesn't work :/
Comment 10 David Lechner 2014-08-05 00:32:27 UTC
I am getting this crash as well. For me, it turned out it is triggered by my kernel boot parameters. I have console=ttyS1, which is a serial terminal, then I also had plymouth.ignore-serial-consoles, which is why I think that the terminal variable is null and causing the crash.

Perhaps there should be some logic added to handle this case.
Comment 11 David Lechner 2014-08-05 04:18:07 UTC
Hmm. I see now that if there is a serial console, then plymouth automatically does text instead of trying to use the framebuffer, so nevermind.
Comment 12 David Lechner 2014-08-06 01:55:39 UTC
Ray, have you thought about this (comment 8) any more? I've dug a little deeper and am understanding this better now. With a little guideance, I might be able to come up with something to get this fixed.

My particular case is using a framebuffer driver on an embedded system, so it does not have the boot_vga attribute.

I patched ply-device-manager.c so that it does not call device_is_for_local_console() and just assumes that it is true. This fixed keyboard not working in bootsplash and it also fixed bug 73585 / bug 66260.

I also came across this at http://www.freedesktop.org/wiki/Software/systemd/multiseat/

> If you are writing a bootup splash tool (like Plymouth), then ignore all seat information completely and make use of all input devices and graphics cards as they become available. Stop using them as soon as the first X server starts up.

I also had an issue with the udev seat rules not being copied to the initrd and causing plymouth to fallback to text mode. Perhaps we should take this advice and not look at the seat tag and it will fix very many problems.
Comment 13 Chris Bainbridge 2016-02-06 17:49:03 UTC
*** Bug 87993 has been marked as a duplicate of this bug. ***
Comment 14 GitLab Migration User 2018-08-07 09:25:08 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/plymouth/plymouth/issues/10.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.