Re: [systemd-devel] Shutdown problems
Nikolaus Rath nikol...@rath.org writes: On 11/13/2014 12:54 PM, Nikolaus Rath wrote: Nikolaus Rath nikol...@rath.org writes: Lennart Poettering lenn...@poettering.net writes: On Sat, 08.11.14 11:16, Nikolaus Rath (nikol...@rath.org) wrote: Please boot with systemd.log_level=debug, then make the machine hang and check what the last things in the logs say. Maybe then paste that somewhere online and post the URL for that here, so that we can have a look. Here's the output (obtained by changing log level and remounting earlier in the debug.sh script): https://dl.dropboxusercontent.com/u/11545826/shutdown.log Thanks for your help! Hmm the logs show that systemd pretty much completed its shutdown. After the message Cannot finalize remaining DM devices, continuing. the only thing that still runs is the shutdown hooks you used to generate this log, plus either a jump back into your initrd (if your initrd supports that) or the reboot() system call. If the latter hangs then it's a kernel bug. reboot -f works fine - could it still be a kernel bug? Please check if there are any other scripts in /lib/systemd/system-shutdown/ that might be at fault here. Nope, none. Please check if your initrd is one of those which support jumping back into the initrd on shutdown. For that check if /run/initramfs/shutdown exists during runtime and is executable. No, /run/initramfs/shutdown does not exist. If so, it's probably an initrd problem, please file a bug against your initrd implementation. You appear to be using LVM, I wouldn't be surprised if LVM is broken here, but I cannot help you debugging this, please contact the LVM maintainers in this case. Is there some indication that LVN is at fault? As I said in my first email, the crucial difference seems to be if an X11 console is active or not: * If I execute systemctl reboot while a text console is active, everything works fine. * If I execute systemctl reboot while the X11 console is active, the system hangs (I tried waiting up to 7 minutes). Furthermore, I am unable to switch to another console with Ctrl+Alt+Fn, the computer becomes unresponsive to the keyboard and the monitor powers down. On which tty/pty systemctl itself is executed does not matter (I tested this by running systemctl in an ssh session from a remote system), it only matters which console is currently active. Some more information: * if I start a debug-shell on a serial port, at some point the shell seems to freeze as well. * if I boot with sysvinit instead of systemd things work fine: - The system reboots even if an X11 console is active - During the shutdown, I can switch between text consoles and see log messages Any ideas? On a whim, I also tried blacklisting the nouveau module and using the integrated graphics with the i915 module instead. It did not make a difference. No one able to help at all? Best, -Nikolaus -- GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F »Time flies like an arrow, fruit flies like a Banana.« ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory
Hi On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier chevalier.ro...@gmail.com wrote: CID#979416 --- src/udev/collect/collect.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c index dc849bd..6cb10fe 100644 --- a/src/udev/collect/collect.c +++ b/src/udev/collect/collect.c @@ -86,12 +86,13 @@ static void usage(void) */ static int prepare(char *dir, char *filename) { -struct stat statbuf; char buf[512]; int fd; +int r; -if (stat(dir, statbuf) 0) -mkdir(dir, 0700); +r = mkdir(dir, 0700); +if (r 0 errno != EEXIST) +return -errno; snprintf(buf, sizeof(buf), %s/%s, dir, filename); So the race you describe is if the directory is removed after we stat() it, but before we use it somewhere down in the code. Applying your patch avoids the stat(), but it still fails if the dir is removed after your mkdir(). So this doesn't fix anything, does it? The code is definitely nicer than before, so I guess I'll apply it, anyway. But I don't see how it would fix anything, but silence a coverity warning. Or am I missing something? Feel free to prove me wrong ;) Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory
Hi On Sun, Nov 16, 2014 at 7:34 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier chevalier.ro...@gmail.com wrote: CID#979416 --- src/udev/collect/collect.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c index dc849bd..6cb10fe 100644 --- a/src/udev/collect/collect.c +++ b/src/udev/collect/collect.c @@ -86,12 +86,13 @@ static void usage(void) */ static int prepare(char *dir, char *filename) { -struct stat statbuf; char buf[512]; int fd; +int r; -if (stat(dir, statbuf) 0) -mkdir(dir, 0700); +r = mkdir(dir, 0700); +if (r 0 errno != EEXIST) +return -errno; snprintf(buf, sizeof(buf), %s/%s, dir, filename); So the race you describe is if the directory is removed after we stat() it, but before we use it somewhere down in the code. Applying your patch avoids the stat(), but it still fails if the dir is removed after your mkdir(). So this doesn't fix anything, does it? The code is definitely nicer than before, so I guess I'll apply it, anyway. But I don't see how it would fix anything, but silence a coverity warning. Or am I missing something? Feel free to prove me wrong ;) One more addition: your code avoids an additional syscall, so yeah, it's nicer. So I applied it now! Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory
2014-11-16 19:39 GMT+01:00 David Herrmann dh.herrm...@gmail.com: Hi On Sun, Nov 16, 2014 at 7:34 PM, David Herrmann dh.herrm...@gmail.com wrote: Hi On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier chevalier.ro...@gmail.com wrote: CID#979416 --- src/udev/collect/collect.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c index dc849bd..6cb10fe 100644 --- a/src/udev/collect/collect.c +++ b/src/udev/collect/collect.c @@ -86,12 +86,13 @@ static void usage(void) */ static int prepare(char *dir, char *filename) { -struct stat statbuf; char buf[512]; int fd; +int r; -if (stat(dir, statbuf) 0) -mkdir(dir, 0700); +r = mkdir(dir, 0700); +if (r 0 errno != EEXIST) +return -errno; snprintf(buf, sizeof(buf), %s/%s, dir, filename); So the race you describe is if the directory is removed after we stat() it, but before we use it somewhere down in the code. Applying your patch avoids the stat(), but it still fails if the dir is removed after your mkdir(). So this doesn't fix anything, does it? Right The code is definitely nicer than before, so I guess I'll apply it, anyway. But I don't see how it would fix anything, but silence a coverity warning. Or am I missing something? Feel free to prove me wrong ;) One more addition: your code avoids an additional syscall, so yeah, it's nicer. So I applied it now! It was the purpose of this patch. The stat() call was useless and we did not check if the mkdir() succeeded. But you are right, it does not fix any TOCTOU, I think I named it this way because of the coverity report. So you can change the commit message if you want :) Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] bus: fix null pointer dereference
Hi On Sun, Nov 9, 2014 at 3:41 PM, Ronny Chevalier chevalier.ro...@gmail.com wrote: CID#1237620 --- src/libsystemd/sd-bus/bus-message.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index be36d9f..edadacf 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -2048,6 +2048,7 @@ static int bus_message_close_variant(sd_bus_message *m, struct bus_container *c) assert(m); assert(c); +assert(c-signature); if (!BUS_MESSAGE_IS_GVARIANT(m)) return 0; @@ -2174,6 +2175,8 @@ _public_ int sd_bus_message_close_container(sd_bus_message *m) { if (c-enclosing != SD_BUS_TYPE_ARRAY) if (c-signature c-signature[c-index] != 0) return -EINVAL; +if (!c-signature c-enclosing == SD_BUS_TYPE_VARIANT) +return -EINVAL; I think we expect c-signature to always be non-NULL. See sd_bus_message_enter_container() and sd_bus_message_open_container(). They call strdup() on the signature unconditionally and I cannot see another place that allocates bus_container objects. I'll leave this to Lennart, as he wrote that code. If c-signature is always non-NULL, we should probably remove the if (c-signature part in the line above. Thanks David m-n_containers--; -- 2.1.3 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel