segfault message supressed with su -c

PATCH Applied Ondrej Oprala 4 participants 6 messages Sep 11, 2012 – Sep 21, 2012 Util-Linux package

Share & tools

Ondrej Oprala

author

2012-09-11 15:01 UTC #0 lore raw

Hi,
I've written a tiny patch that should fix this issue with su:
https://bugzilla.redhat.com/show_bug.cgi?id=747592.
None of the shells' output propagates back
to the shell where su was called (except TCSH, which reports segfault to stdout; I've tested this with sh,ksh,zsh,bash).
So, with this patch, su should print a warning if WIFSIGNALED(status) && WCOREDUMP(status) is true.

Cheers,
Ondrej

Quoted 1 line

From d4776f487e283649d65be3f3865141a0171a80e7 Mon Sep 17 00:00:00 2001

From: Ondrej Oprala <[email protected]>
Date: Tue, 11 Sep 2012 16:39:17 +0200
Subject: [PATCH] su: add segmentation fault reporting of the child process

Child processes that ended with segmentation fault previously indicated this with return status only. The report is now more verbose if core dump is allowed.

---
 login-utils/su-common.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index 23ad57d..b0dbed2 100644
@@ -300,7 +300,11 @@ create_watching_parent (void)
 	}
       if (pid != (pid_t)-1)
 	if (WIFSIGNALED (status))
-	  status = WTERMSIG (status) + 128;
+	  {
+	    status = WTERMSIG (status) + 128;
+	    if (WCOREDUMP (status))
+	      fprintf (stderr, _("Segmentation fault (core dumped)\n"));
+	  }
 	else
 	  status = WEXITSTATUS (status);
       else

Signature · 1 line

1.7.11.4

Voelker, Bernhard

2012-09-11 16:31 UTC #1 lore raw

Quoted 8 lines · Ondrej Oprala

Hi,
I've written a tiny patch that should fix this issue with su:
https://bugzilla.redhat.com/show_bug.cgi?id=747592.
None of the shells' output propagates back
to the shell where su was called (except TCSH, which reports segfault to stdout; I've tested this with sh,ksh,zsh,bash).
So, with this patch, su should print a warning if WIFSIGNALED(status) && WCOREDUMP(status) is true.

Good catch!

Quoted 1 line

+	      fprintf (stderr, _("Segmentation fault (core dumped)\n"));

Maybe the error message should point out that it
was not su which dumped core ... ;-)

Have a nice day,
Berny

Pádraig Brady

2012-09-11 18:02 UTC #2 lore raw

Quoted 10 lines · Bernhard

Quoted 8 lines · Ondrej Oprala
Hi,
I've written a tiny patch that should fix this issue with su:
https://bugzilla.redhat.com/show_bug.cgi?id=747592.
None of the shells' output propagates back
to the shell where su was called (except TCSH, which reports segfault to stdout; I've tested this with sh,ksh,zsh,bash).
So, with this patch, su should print a warning if WIFSIGNALED(status)&& WCOREDUMP(status) is true.

Quoted 6 lines

Good catch!
login-utils/su-common.c create_watching_parent (void) · line 306
Quoted 1 line
login-utils/su-common.c +1 -0
+	      fprintf (stderr, _("Segmentation fault (core dumped)\n"));
Maybe the error message should point out that it
was not su which dumped core ... ;-)

Maybe not. Consider:

monitor_1 timeout 2 'su -c command'

You might as well just say "core dumped",
and leave the investigation as to what
cored exactly to inspection of the core file.

cheers,
Pádraig.

Pádraig Brady

2012-09-11 17:19 UTC #3 lore raw

In reply to Ondrej Oprala

Quoted 35 lines · Ondrej Oprala

Hi,
I've written a tiny patch that should fix this issue with su:
https://bugzilla.redhat.com/show_bug.cgi?id=747592.
None of the shells' output propagates back
to the shell where su was called (except TCSH, which reports segfault to stdout; I've tested this with sh,ksh,zsh,bash).
So, with this patch, su should print a warning if WIFSIGNALED(status) && WCOREDUMP(status) is true.
Cheers,
Ondrej
DIFF
Quoted 1 line
From d4776f487e283649d65be3f3865141a0171a80e7 Mon Sep 17 00:00:00 2001
From: Ondrej Oprala<[email protected]>
Date: Tue, 11 Sep 2012 16:39:17 +0200
Subject: [PATCH] su: add segmentation fault reporting of the child process
Child processes that ended with segmentation fault previously indicated this with return status only. The report is now more verbose if core dump is allowed.
---
  login-utils/su-common.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
login-utils/su-common.c +0 -0
index 23ad57d..b0dbed2 100644
@@ -300,7 +300,11 @@ create_watching_parent (void)
  	}
        if (pid != (pid_t)-1)
  	if (WIFSIGNALED (status))

Quoted 10 lines

-	  status = WTERMSIG (status) + 128;
+	  {
+	    status = WTERMSIG (status) + 128;
+	    if (WCOREDUMP (status))
+	      fprintf (stderr, _("Segmentation fault (core dumped)\n"));
+	  }
  	else
  	  status = WEXITSTATUS (status);
        else
-- 1.7.11.4

You'll need to key on WTERMSIG (status). "Segmentation fault" will not always be appropriate. You'll get a core just with Ctrl-\ (SIGQUIT) too for example. Something along the lines of:

"%s (core dumped)", strsignal (WTERMSIG (status))

cheers,
Pádraig.

Ondrej Oprala

author

2012-09-12 10:22 UTC #4 lore raw

Quoted 9 lines

You'll need to key on WTERMSIG (status). "Segmentation fault" will not always be appropriate. You'll get a core just with Ctrl-\ (SIGQUIT) too for example. Something along the lines of:
"%s (core dumped)", strsignal (WTERMSIG (status))
cheers,
Pádraig.

I've redone the patch to reflect your proposition :) . Cheers,
Ondrej.

From 65e73591fce2fa961bf6de1cbc934f6aa7a22745 Mon Sep 17 00:00:00 2001
From: Ondrej Oprala <[email protected]>
Date: Tue, 11 Sep 2012 16:39:17 +0200
Subject: [PATCH] su: add segmentation fault reporting of the child process
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Child processes that ended with segmentation fault previously indicated this with return status only. The report is now more verbose if core dump is allowed.

Improved-by: Pádraig Brady.

---
 login-utils/su-common.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

index 23ad57d..2749303 100644
@@ -300,7 +300,12 @@ create_watching_parent (void)
 	}
       if (pid != (pid_t)-1)
 	if (WIFSIGNALED (status))
-	  status = WTERMSIG (status) + 128;
+	  {
+	    status = WTERMSIG (status) + 128;
+	    if (WCOREDUMP (status))
+	      fprintf (stderr, _("%s (core dumped)\n"),
+                 strsignal (WTERMSIG (status)));
+	  }
 	else
 	  status = WEXITSTATUS (status);
       else

Signature · 1 line

1.7.11.4

Karel Zak

maintainer Applied

2012-09-21 10:39 UTC #5 lore raw

Quoted 2 lines · Ondrej Oprala

 login-utils/su-common.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Applied, thanks.

Signature · 2 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com