Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-10-17 Thread Yves-Alexis Perez
On Mon, 2016-10-17 at 16:48 +0100, Ian Jackson wrote:
> To the lightdm maintainers: I intend to NMU (to DELAYED/7) to apply
> the patch, unless you object.

Yeah, please refrain. What I needed is an upstream comment on this, which
didn't happen. I'll ping them again on the upstream tracker because it seems
that it's been forgotten there too.

Regards,
-- 
Yves-Alexis

signature.asc
Description: This is a digitally signed message part


Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-10 Thread Ian Jackson
Yves-Alexis Perez writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE 
ignored in session"):
> Here it is: https://bugs.launchpad.net/lightdm/+bug/1579867

Thanks.

Ian.



Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-09 Thread Yves-Alexis Perez
control: forwarded -1 https://bugs.launchpad.net/lightdm/+bug/1579867

On lun., 2016-05-09 at 18:17 +0100, Ian Jackson wrote:
> Yves-Alexis Perez writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE
> ignored in session"):
> > 
> > Thanks for the investigation and the patch, I'll forward this upstream for
> > comments/integration.
> Thanks.  Can you post a reference to the upstream discussion here in
> the Debian bug, as appropriate ?  I'd like to take an interest in the
> upstream conversation.

Here it is: https://bugs.launchpad.net/lightdm/+bug/1579867
-- 
Yves-Alexis



signature.asc
Description: This is a digitally signed message part


Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-09 Thread Ian Jackson
Yves-Alexis Perez writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE 
ignored in session"):
> Thanks for the investigation and the patch, I'll forward this upstream for
> comments/integration.

Thanks.  Can you post a reference to the upstream discussion here in
the Debian bug, as appropriate ?  I'd like to take an interest in the
upstream conversation.

Thanks,
Ian.



Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-09 Thread Yves-Alexis Perez
On dim., 2016-05-08 at 18:39 +0100, Ian Jackson wrote:
> Control: tags -1 + patch
> 
> Ian Jackson writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE
> ignored in session"):
> > 
> > Do you want me to send you a patch ?
> The patch is straightforward.  See attached.
> 
> Also a fixed version of the glibc patch which gets the checking for
> signals other than PIPE right.

Thanks for the investigation and the patch, I'll forward this upstream for
comments/integration.

Regards,
-- 
Yves-Alexis



signature.asc
Description: This is a digitally signed message part


Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-08 Thread Ian Jackson
Control: tags -1 + patch

Ian Jackson writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored 
in session"):
> Do you want me to send you a patch ?

The patch is straightforward.  See attached.

Also a fixed version of the glibc patch which gets the checking for
signals other than PIPE right.

Regards,
Ian.

diff --git a/debian/changelog b/debian/changelog
index df6c1aa..551697f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+lightdm (1.18.1-1+iwj1) UNRELEASED; urgency=medium
+
+  * Reset SIGPIPE in session_child_run.  Closes:#823460.
+  * Reset SIGPIPE in process_start (affects the X server, for example).
+
+ -- Ian Jackson <ijack...@chiark.greenend.org.uk>  Sun, 08 May 2016 17:50:01 
+0100
+
 lightdm (1.18.1-1) unstable; urgency=medium
 
   * New upstream bugfix release.
diff --git a/src/process.c b/src/process.c
index d9b7eb9..dbe86c8 100644
--- a/src/process.c
+++ b/src/process.c
@@ -213,6 +213,8 @@ process_start (Process *process, gboolean block)
 pid = fork ();
 if (pid == 0)
 {
+signal(SIGPIPE, SIG_DFL); /* Undo glib2.0's SIG_IGN */
+
 /* Do custom setup */
 if (process->priv->run_func)
 process->priv->run_func (process, process->priv->run_func_data);
diff --git a/src/session-child.c b/src/session-child.c
index f3a5e1b..981cfb7 100644
--- a/src/session-child.c
+++ b/src/session-child.c
@@ -676,6 +676,8 @@ session_child_run (int argc, char **argv)
 child_pid = fork ();
 if (child_pid == 0)
 {
+signal(SIGPIPE, SIG_DFL); /* Undo glib2.0's SIG_IGN */
+
 /* Make this process its own session */
 if (setsid () < 0)
 _exit (errno);
diff --git a/csu/init-first.c b/csu/init-first.c
index b3bacdd..adc2785 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -39,6 +39,92 @@ int __libc_argc attribute_hidden;
 char **__libc_argv attribute_hidden;
 
 
+#include 
+#include 
+#include 
+#include 
+
+static void
+__libc_check_sigpipe (const char *argv0)
+{
+  static const int sigs[] = { SIGPIPE, SIGTERM, SIGALRM, 9 };
+  const int *sigp;
+  int sig;
+  struct sigaction sa;
+  sigset_t mask;
+  int r;
+  const char *oddity;
+
+  sig = 0;
+  r = sigprocmask(SIG_UNBLOCK, 0, );
+  if (r) { oddity = strerror(errno); goto bad; }
+
+  for (sigp = sigs; (sig = *sigp); sigp++) {
+r = sigaction(sig, 0, );
+if (r) { oddity = strerror(errno); goto bad; }
+if (sa.sa_handler == SIG_IGN) { oddity = "SIG_IGN"; goto bad; }
+if (sigismember(, sig)) { oddity = "blocked"; goto bad; }
+  }
+  return;
+
+ bad:;
+  int logfd = -1;
+  FILE *logf = 0;
+
+  logfd = open("/var/log/exec-sigignblock.log", O_APPEND|O_WRONLY);
+  if (logfd < 0)
+if (errno == ENOENT || errno == EACCES || errno == EPERM)
+  return;
+
+  logf = fdopen(logfd, "a");
+  if (!logf) goto fail;
+  logfd = -1; /* eaten by fdopen */
+
+  unsigned long ourpid = getpid();
+  unsigned long ppid = getppid();
+  char parentbuf[100];
+
+  snprintf(parentbuf, sizeof(parentbuf), "/proc/%lu/exe", ppid);
+  r = readlink(parentbuf, parentbuf, sizeof(parentbuf)-1);
+  if (r < 0) {
+const char *m = strerror(errno);
+strncpy(parentbuf, m, sizeof(parentbuf)-1);
+parentbuf[sizeof(parentbuf)-1] = 0;
+  } else if (r == 0) {
+strcpy(parentbuf, "\"\"");
+  } else {
+parentbuf[r] = 0;
+  }
+
+  time_t now;
+  now = time(NULL);
+  if (now == (time_t)-1) { errno = EIO; goto fail; }
+
+  struct tm *gmt = gmtime();
+  if (!gmt) goto fail;
+
+  r = fprintf(logf, "%04d-%02d-%02d %02d:%02d:%02d UTC:"
+ "%s[%lu] execd oddly (parent %s[%lu]): %s: %s\n",
+ gmt->tm_year+1900, gmt->tm_mon, gmt->tm_mday,
+ gmt->tm_hour, gmt->tm_min, gmt->tm_sec,
+ argv0, ourpid,
+ parentbuf, ppid,
+ sig ? strsignal(sig) : "sigprocmask", oddity);
+  if (r < 0) goto fail;
+
+  r = fclose(logf);
+  logf = 0;
+  if (r) goto fail;
+
+  return;
+
+ fail:
+  perror("__libc_check_sigpipe report oddity");
+  if (logf) fclose(logf);
+  if (logfd>=0) close(logfd);
+  return;
+}
+
 void
 __libc_init_first (int argc, char **argv, char **envp)
 {
@@ -96,6 +182,8 @@ _init (int argc, char **argv, char **envp)
 #if defined SHARED && !defined NO_CTORS_DTORS_SECTIONS
   __libc_global_ctors ();
 #endif
+
+  __libc_check_sigpipe (argv[0]);
 }
 
 /* This function is defined here so that if this file ever gets into
diff --git a/debian/changelog b/debian/changelog
index f552f26..33a6254 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+glibc (2.22-7+iwj) UNRELEASED; urgency=medium
+
+  * SIGPIPE tracking
+
+ -- Ian Jackson <ijack...@chiark.greenend.org.uk>  Sun, 08 May 2016 12:43:46 
+0100
+
 glibc (2.22-7) unstable; urgency=medium
 
   [ Samuel Thibault ]
diff --git a/debian/shlibs-add-udebs b/debian/shlibs-add-udebs
old mode 100644
new mode 100755


Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-08 Thread Ian Jackson
Ian Jackson writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored 
in session"):
> As previously discussed here, the ignoring of SIGPIPE doesn't seem to
> be done explicitly in the lightdm source code.  Having established
> that it is (almost certainly) the lightdm process which is responsible
> I will now attempt to find that out.

It seems to be done by glib.  I found this in the glib2.0 source code:

 * Note that creating a #GSocket causes the signal %SIGPIPE to be
 * ignored for the remainder of the program. If you are writing a
 * command-line utility that uses #GSocket, you may need to take into
 * account the fact that your program will not automatically be killed
 * if it tries to write to %stdout after it has been closed.

(gio/gsocket.c from 2.48.0-1).  This seems also to be documented in
docs/reference/gio/html/GSocket.html.

glib2.0 provides g_spawn* functions which unconditionally put SIGPIPE
back.  (glib/gspawn.c)

I think glib does this because it's difficult to portably avoid
SIGPIPE from sockets as a library which might be running in a
multithreaded program.  I have some sympathy with this view.  I don't
want to call this a bug in glib.

The upshot is that programs which:
  1. use glib;
  2. spawn other programs other than by using glib_spawn
need to reset SIGPIPE themselves.  (The same might be true of other
libraries besides glib).

So I think this is indeed a bug in lightdm.

Do you want me to send you a patch ?

Thanks,
Ian.

PS here is an example backtrace from lightdm.  AFAICT that SIGPIPE is
set to IGN several times, in various different processes in lightdm's
process farm.  All the backtraces I obtained (by playing about with
`set follow-fork-mode') seemed to implicate gio.

(gdb) bt
#0  __bsd_signal (sig=13, handler=0x1) at ../sysdeps/posix/signal.c:36
#1  0x77aca42b in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#2  0x7783022d in g_type_class_ref () from 
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#3  0x778173a4 in g_object_new_valist () from 
/usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0
#4  0x77ab19fe in g_initable_new_valist () from 
/usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#5  0x77ab1ac6 in g_initable_new () from 
/usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#6  0x77acb138 in g_socket_new () from 
/usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#7  0x77ad10a8 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#8  0x77ad22b3 in g_socket_client_connect () from 
/usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#9  0x77b1b05d in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#10 0x77b1b5f8 in g_dbus_address_get_stream_sync () from 
/usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#11 0x77b2c057 in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#12 0x77a89f9e in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#13 0x77adb2fd in ?? () from /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0
#14 0x7756050e in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#15 0x7755fb75 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#16 0x7676d454 in start_thread (arg=0x748ce700) at 
pthread_create.c:334
#17 0x764aaeed in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109
(gdb)



Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-08 Thread Ian Jackson
Ian Jackson writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored 
in session"):
> I have a plan for how to track this down further.  I will get back to
> you.

I applied the attached patch below to my libc, and created the logfile
/var/log/exec-sigignblock.log world-writeable, and rebooted.  An
extract from the logfile is also attached.

The first line which mentions lightdm is this one:

  2016-04-08 12:15:12 UTC:/bin/sh[3344] execd oddly (parent
  /usr/sbin/lightdm[3294]): Broken pipe: SIG_IGN

Note that that shell process almost immmediately spawns Xorg.

This allows us to conclude:

1. When lightdm is first execd by whatever spawns it, it does NOT
   inherit SIGPIPE set to SIG_IGN.  If this weren't the case there
   would be a message about lightdm being execd oddly, before lightdm
   itself execs anything else.

2. lightdm is definitely executing Xorg with SIGPIPE set to SIG_IGN.
   This is IMO a bug.  It is a less serious bug than running the
   user's session this way because the X server probablhy doesn't
   spawn that many other processes (so that the bug can only influence
   a limited set of processes), and the X server probably runs with
   SIGPIPE ignored anyway (so that while this is wrong, there is no
   actual adverse consequence).

Further on, we see;

  2016-04-08 12:15:22 UTC:/bin/bash[4164] execd oddly (parent Permission
  denied[4155]): Broken pipe: SIG_IGN

and then 4164 execs a lot of things which are part of my .xsession.

The prior report about pid 4155 is

  2016-04-08 12:15:20 UTC:lightdm[4155] execd oddly (parent
  /usr/sbin/lightdm[3294]): Broken pipe: SIG_IGN

Ie, 4164 was spawned by lightdm, and is running as a different user
(so the readlink of /proc/4155/exe failed).

lightdm is of course entitled to exec itself with SIGPIPE ignored if
it so chooses, but that (almost conclusively) proves that it is
the lightdm process which is at fault.

The only way in which it could be someone else's fault is if lightdm
forked 4155, and execed some other program with PIPE=DFL (which
wouldn't show up in my log), and that other program set PIPE=IGN and
in turn execed lightdm.  Then the bug would be in that other
program.  I don't think this is at all likely.

As previously discussed here, the ignoring of SIGPIPE doesn't seem to
be done explicitly in the lightdm source code.  Having established
that it is (almost certainly) the lightdm process which is responsible
I will now attempt to find that out.


Incidentally, the log contains a lot of messages like this:

  2016-04-08 12:15:12 UTC:/sbin/wpa_supplicant[3350] execd oddly (parent
  /usr/bin/dbus-daemon[3349]): Broken pipe: SIG_IGN

I think this is a separate bug.  Unless lightdm is spawned from dbus
somehow, I don't think this can be responsible for my session having
PIPE=IGN.  Furthermore, if this was the root cause of the bug I would
expect to see a complaint about the initial parent lightdm itself
being execed oddly.


Watch this space for more information.

Ian.

diff --git a/csu/init-first.c b/csu/init-first.c
index b3bacdd..751fc5a 100644
--- a/csu/init-first.c
+++ b/csu/init-first.c
@@ -39,6 +39,92 @@ int __libc_argc attribute_hidden;
 char **__libc_argv attribute_hidden;
 
 
+#include 
+#include 
+#include 
+#include 
+
+static void
+__libc_check_sigpipe (const char *argv0)
+{
+  static const int sigs[] = { SIGPIPE, SIGTERM, SIGALRM, 9 };
+  const int *sigp;
+  int sig;
+  struct sigaction sa;
+  sigset_t mask;
+  int r;
+  const char *oddity;
+
+  sig = 0;
+  r = sigprocmask(SIG_UNBLOCK, 0, );
+  if (r) { oddity = strerror(errno); goto bad; }
+
+  for (sigp = sigs; (sig = *sigp); sigp++) {
+r = sigaction(SIGPIPE, 0, );
+if (r) { oddity = strerror(errno); goto bad; }
+if (sa.sa_handler == SIG_IGN) { oddity = "SIG_IGN"; goto bad; }
+if (sigismember(, sig)) { oddity = "blocked"; goto bad; }
+  }
+  return;
+
+ bad:;
+  int logfd = -1;
+  FILE *logf = 0;
+
+  logfd = open("/var/log/exec-sigignblock.log", O_APPEND|O_WRONLY);
+  if (logfd < 0)
+if (errno == ENOENT || errno == EACCES || errno == EPERM)
+  return;
+
+  logf = fdopen(logfd, "a");
+  if (!logf) goto fail;
+  logfd = -1; /* eaten by fdopen */
+
+  unsigned long ourpid = getpid();
+  unsigned long ppid = getppid();
+  char parentbuf[100];
+
+  snprintf(parentbuf, sizeof(parentbuf), "/proc/%lu/exe", ppid);
+  r = readlink(parentbuf, parentbuf, sizeof(parentbuf)-1);
+  if (r < 0) {
+const char *m = strerror(errno);
+strncpy(parentbuf, m, sizeof(parentbuf)-1);
+parentbuf[sizeof(parentbuf)-1] = 0;
+  } else if (r == 0) {
+strcpy(parentbuf, "\"\"");
+  } else {
+parentbuf[r] = 0;
+  }
+
+  time_t now;
+  now = time(NULL);
+  if (now == (time_t)-1) { errno = EIO; goto fail; }
+
+  struct tm *gmt = gmtime();
+  if (!gmt) goto fail;
+
+  r = fprintf(logf, "%04d-%02d-%02d %02d:%02d:%02d UTC:"
+ "

Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-06 Thread Ian Jackson
Yves-Alexis Perez writes ("Re: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE 
ignored in session"):
> control: severity -1 important
...
> That's very much false in my opinion and experience. A program definitely
> *doesn't* know the signal dispositions it starts with, so if it *needs*
> something specific, it'd better do it itself. It might be basic courtesy not
> to mess up with signals for your children, but they definitely can't have any
> expectation.

I think it makes more sense for me to argue about this with whoever
turns out to be maintainer for the responsible code...

> > zealot:~> perl -e 'print $SIG{PIPE},"\n"'
> > IGNORE
> > zealot:~>
> 
> Well, in my session (Xfce) signal dispositions are reset anyway, so
> SIGPIPE is not ignored.

I think this is because xfce4-terminal sets SIGPIPE to IGN for its own
use, and then resets it when it runs its child, masking the problem.

> Also note that lightdm by itself doesn't setup the signal
> dispositions so it's likely to happen somewhere in the underlying
> libs. Any help finding where would be appreciated.

I have a plan for how to track this down further.  I will get back to
you.

Ian.



Bug#823460: [Pkg-xfce-devel] Bug#823460: lightdm: SIGPIPE ignored in session

2016-05-06 Thread Yves-Alexis Perez
control: severity -1 important

On jeu., 2016-05-05 at 00:31 +0100, Ian Jackson wrote:
> All Unix programs are entitled to assume that they start with
> reasonable signal dispositions, which (with a few exceptions) means
> everything set to SIG_DFL.

That's very much false in my opinion and experience. A program definitely
*doesn't* know the signal dispositions it starts with, so if it *needs*
something specific, it'd better do it itself. It might be basic courtesy not
to mess up with signals for your children, but they definitely can't have any
expectation.

>   I have marked this bug "serious" on the
> grounds that I think this is a release-critical bug.

I disagree here too.

> In an xterm:
> 
> zealot:~> perl -e 'print $SIG{PIPE},"\n"'
> IGNORE
> zealot:~>

Well, in my session (Xfce) signal dispositions are reset anyway, so SIGPIPE is
not ignored.

Also note that lightdm by itself doesn't setup the signal dispositions so it's
likely to happen somewhere in the underlying libs. Any help finding where
would be appreciated.

Regards,
-- 
Yves-Alexis



signature.asc
Description: This is a digitally signed message part