[Pkg-sysvinit-devel] Bug#763869: sysvinit-utils: sulogin segfault after `cannot open password database!`

[Paul Menzel]

Control: severity -1 important
Control: tags -1 patch


Dear Debian folks,


Am Mittwoch, den 08.10.2014, 17:45 +0200 schrieb Axel Beckert:

> Pugachev Anatoly wrote:
> > Ahh, sorry, can't reproduce on the next boot log. 
> 
> But it is there. I ran into it on a freshly bootstrapped Xen DomU with
> Debian Sid and currently can reproduce it easily again and again:
> 
> Begin: Loading essential drivers ... done.
> Begin: Running /scripts/init-premount ... done.
> Begin: Mounting root file system ... Begin: Running /scripts/local-top ... done.
> Begin: Running /scripts/local-premount ... done.
> Begin: Checking root file system ... fsck from util-linux 2.25.1
> fsck: error 2 (No such file or directory) while executing fsck.ext3 for /dev/xvda2
> fsck exited with status code 8
> done.
> Failure: An automatic file system check (fsck) of the root filesystem failed.
> A manual fsck must be performed, then the system restarted.
> The fsck should be performed in maintenance mode with the
> root filesystem mounted in read-only mode.
> Warning: The root filesystem is currently mounted in read-only mode.
> A maintenance shell will now be started.
> After performing system maintenance, press CONTROL-D
> to terminate the maintenance shell and restart the system.
> sulogin: cannot open password database!
> [    3.563041] sulogin[84]: segfault at 8 ip 00000000004018e0 sp 00007fffd88d8330 error 4 in sulogin[400000+3000]
> Segment violation
> Failure: Attempt to start maintenance shell failed.
> Will restart in 5 seconds.
> [    8.571169] reboot: Restarting system

on one of my systems, after replacing `initramfs-tools` with `dracut`
and going back to `initramfs-tools` for some reason `fsck.ext4` is not
included in the initrd image causing the routine filesystem check to
fail. The maintenance shell does not show up though and the after the
printing of the message and the delay of two seconds the boot continues.

> There might be other things broken (which I'm hunting down currently),
> but sulogin must not segfault.

Please take a look at the patch below. I am not sure about the correct
fix. Does `return 1` make sense. Does anything check this? Should the
code continue and try to show a password prompt?

The next question is, why the root entry is not found in the database.

You can apply the patch by saving my email as an mbox file and running
`git am --scissors the/path/to/patch.mbox`.

[…]


Thanks,

Paul

---------------------- 8< ------------------ >8 ----------------------

From 259def1ee14e9a4fbf70d187550f8b45f92992dd Mon Sep 17 00:00:00 2001
From: Paul Menzel <paulepanter at users.sourceforge.net>
Date: Tue, 16 Dec 2014 22:34:33 +0100
Subject: [PATCH] sulogin.c: Return if password database cannot be opened to avoid crash

Currently when the maintenance console should open and the password
database cannot be opened the process `sulogin` crashes with a
segmentation fault [1].

	sulogin: cannot open password database!
	[    3.563041] sulogin[84]: segfault at 8 ip 00000000004018e0 sp 00007fffd88d8330 error 4 in sulogin[400000+3000]
	Segment violation

The code calls the function `getrootpwent()` to get the root password
entry. The function should return a passwd struct and `NULL` on failure.

The return value is actually checked for `NULL` but it just prints a
message in this case and tries to continue normally, which is of course
not possible as later on the struct members are accessed.

Therefore just return 0 in case `pwd` is `NULL`.

Note: The sleep of two seconds is also questionable. Unfortunately there
is no comment explaining what the reason for it is.

Closes: #763869 (sysvinit-utils: sulogin segfault)

[1] https://bugs.debian.org/763869
---
 debian/changelog | 7 +++++++
 src/sulogin.c    | 1 +
 2 files changed, 8 insertions(+)

diff --git a/debian/changelog b/debian/changelog
index 4511465..9ba1a26 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+sysvinit (2.88dsf-58.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Fix crash when password database is not available (Closes: 763869)
+
+ -- Paul Menzel <paulepanter at users.sourceforge.net>  Tue, 16 Dec 2014 23:41:45 +0100
+
 sysvinit (2.88dsf-58) unstable; urgency=low
 
   * Fix typo in invoke-rc.d breaking upstart installations (Closes:
diff --git a/src/sulogin.c b/src/sulogin.c
index 7537654..d1cdebc 100644
--- a/src/sulogin.c
+++ b/src/sulogin.c
@@ -584,6 +584,7 @@ int main(int argc, char **argv)
 	if ((pwd = getrootpwent(opt_e)) == NULL) {
 		fprintf(stderr, "sulogin: cannot open password database!\n");
 		sleep(2);
+		return 1;
 	}
 
 	/*
-- 
2.1.3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: This is a digitally signed message part
URL: <http://lists.alioth.debian.org/pipermail/pkg-sysvinit-devel/attachments/20141216/02a58e18/attachment.sig>