Re: [systemd-devel] Shutdown problems

2014-11-16 Thread Nikolaus Rath
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

2014-11-16 Thread David Herrmann
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

2014-11-16 Thread David Herrmann
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 Thread Ronny Chevalier
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

2014-11-16 Thread David Herrmann
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