Bug 102834 - [patch] mail(1) hangs on the sigsuspend system call in popen.c
Summary: [patch] mail(1) hangs on the sigsuspend system call in popen.c
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-03 22:00 UTC by longwitz
Modified: 2019-02-18 18:24 UTC (History)
2 users (show)

See Also:


Attachments
patch.popen.c.diff (675 bytes, patch)
2009-06-10 17:36 UTC, longwitz
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description longwitz 2006-09-03 22:00:39 UTC
Sometimes the command /usr/bin/mail -s subject mailadr hangs on the
sigsuspend system call in popen.c. This happens when the function
findchild() running from wait_child() is interrupted by sigchild()
running findchild() again. This happens when the child started just
before exits too quickly.

Fix: 

In popen.c (functions free_child() and wait_child()) the line
    struct child *cp = findchild(pid);
must be moved after the 3 sigfunction-calls.
Comment 1 longwitz 2009-06-10 17:36:43 UTC
The described problem still occurs in all FreeBSD versions especially on SMP 
machines. The appended patch for 6.4 Stable works without problems for more 
than 3 years.

Dr. Andreas Longwitz  Data Service GmbH, Beethovenstr. 2A, 23617 Stockelsdorf
Comment 2 Mark Linimon freebsd_committer freebsd_triage 2009-06-10 21:41:55 UTC
State Changed
From-To: open->analyzed

problem confirmed to still exist.
Comment 3 Eitan Adler freebsd_committer freebsd_triage 2012-05-06 15:46:39 UTC
Responsible Changed
From-To: freebsd-bugs->eadler

I'll take it.
Comment 4 Eitan Adler freebsd_committer freebsd_triage 2012-05-06 15:47:01 UTC
State Changed
From-To: analyzed->open

need to check patch
Comment 5 Eitan Adler freebsd_committer freebsd_triage 2012-05-17 00:52:39 UTC
State Changed
From-To: open->analyzed

awaiting approval
Comment 6 dfilter service freebsd_committer 2012-05-30 04:55:57 UTC
Author: eadler
Date: Wed May 30 03:55:44 2012
New Revision: 236286
URL: http://svn.freebsd.org/changeset/base/236286

Log:
  Fix likely race condition if wait_child() is interrupted by sigchild()
  
  PR:		bin/102834
  Submitted by:	Andreas Longwitz <longwitz@incore.de>
  Approved by:	cperciva
  MFC after:	2 weeks

Modified:
  head/usr.bin/mail/popen.c

Modified: head/usr.bin/mail/popen.c
==============================================================================
--- head/usr.bin/mail/popen.c	Wed May 30 03:54:10 2012	(r236285)
+++ head/usr.bin/mail/popen.c	Wed May 30 03:55:44 2012	(r236286)
@@ -336,12 +336,14 @@ int
 wait_child(int pid)
 {
 	sigset_t nset, oset;
-	struct child *cp = findchild(pid);
+	struct child *cp;
 
 	(void)sigemptyset(&nset);
 	(void)sigaddset(&nset, SIGCHLD);
 	(void)sigprocmask(SIG_BLOCK, &nset, &oset);	
 
+	cp = findchild(pid);
+
 	while (!cp->done)
 		(void)sigsuspend(&oset);
 	wait_status = cp->status;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2012-05-30 05:17:10 UTC
State Changed
From-To: analyzed->patched

committed, awaiting MFC
Comment 8 dfilter service freebsd_committer 2012-06-13 04:41:09 UTC
Author: eadler
Date: Wed Jun 13 03:40:59 2012
New Revision: 236984
URL: http://svn.freebsd.org/changeset/base/236984

Log:
  MFC r236286:
  	Fix likely race condition if wait_child() is interrupted by sigchild()
  
  PR:		bin/102834
  Approved by:	cperciva (implicit)

Modified:
  stable/9/usr.bin/mail/popen.c
Directory Properties:
  stable/9/usr.bin/mail/   (props changed)

Modified: stable/9/usr.bin/mail/popen.c
==============================================================================
--- stable/9/usr.bin/mail/popen.c	Wed Jun 13 03:34:42 2012	(r236983)
+++ stable/9/usr.bin/mail/popen.c	Wed Jun 13 03:40:59 2012	(r236984)
@@ -336,12 +336,14 @@ int
 wait_child(int pid)
 {
 	sigset_t nset, oset;
-	struct child *cp = findchild(pid);
+	struct child *cp;
 
 	(void)sigemptyset(&nset);
 	(void)sigaddset(&nset, SIGCHLD);
 	(void)sigprocmask(SIG_BLOCK, &nset, &oset);	
 
+	cp = findchild(pid);
+
 	while (!cp->done)
 		(void)sigsuspend(&oset);
 	wait_status = cp->status;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 9 Eitan Adler freebsd_committer freebsd_triage 2012-06-13 04:41:17 UTC
State Changed
From-To: patched->closed

Committed. Thanks!
Comment 10 dfilter service freebsd_committer 2012-06-13 04:41:31 UTC
Author: eadler
Date: Wed Jun 13 03:41:22 2012
New Revision: 236985
URL: http://svn.freebsd.org/changeset/base/236985

Log:
  MFC r236286:
  	Fix likely race condition if wait_child() is interrupted by sigchild()
  
  PR:		bin/102834
  Approved by:	cperciva (implicit)

Modified:
  stable/8/usr.bin/mail/popen.c
Directory Properties:
  stable/8/usr.bin/mail/   (props changed)

Modified: stable/8/usr.bin/mail/popen.c
==============================================================================
--- stable/8/usr.bin/mail/popen.c	Wed Jun 13 03:40:59 2012	(r236984)
+++ stable/8/usr.bin/mail/popen.c	Wed Jun 13 03:41:22 2012	(r236985)
@@ -364,12 +364,14 @@ wait_child(pid)
 	int pid;
 {
 	sigset_t nset, oset;
-	struct child *cp = findchild(pid);
+	struct child *cp;
 
 	(void)sigemptyset(&nset);
 	(void)sigaddset(&nset, SIGCHLD);
 	(void)sigprocmask(SIG_BLOCK, &nset, &oset);	
 
+	cp = findchild(pid);
+
 	while (!cp->done)
 		(void)sigsuspend(&oset);
 	wait_status = cp->status;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 11 dfilter service freebsd_committer 2012-06-13 04:41:55 UTC
Author: eadler
Date: Wed Jun 13 03:41:42 2012
New Revision: 236986
URL: http://svn.freebsd.org/changeset/base/236986

Log:
  MFC r236286:
  	Fix likely race condition if wait_child() is interrupted by sigchild()
  
  PR:		bin/102834
  Approved by:	cperciva (implicit)

Modified:
  stable/7/usr.bin/mail/popen.c
Directory Properties:
  stable/7/usr.bin/mail/   (props changed)

Modified: stable/7/usr.bin/mail/popen.c
==============================================================================
--- stable/7/usr.bin/mail/popen.c	Wed Jun 13 03:41:22 2012	(r236985)
+++ stable/7/usr.bin/mail/popen.c	Wed Jun 13 03:41:42 2012	(r236986)
@@ -364,12 +364,14 @@ wait_child(pid)
 	int pid;
 {
 	sigset_t nset, oset;
-	struct child *cp = findchild(pid);
+	struct child *cp;
 
 	(void)sigemptyset(&nset);
 	(void)sigaddset(&nset, SIGCHLD);
 	(void)sigprocmask(SIG_BLOCK, &nset, &oset);	
 
+	cp = findchild(pid);
+
 	while (!cp->done)
 		(void)sigsuspend(&oset);
 	wait_status = cp->status;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 12 longwitz 2015-04-22 16:01:19 UTC
The problem still exists, because the commit r236286 has solved the described problem in the function wait_child() of the source popen.c but not in the function free_child().
Comment 13 Eitan Adler freebsd_committer freebsd_triage 2015-04-22 16:13:51 UTC
re-open.
Comment 14 longwitz 2019-02-18 18:24:57 UTC
The problem in this PR is the fact that in the functions wait_child() and free_child() of source popen.c the call of findchild() must be done after the call of the sigfunctions. For wait_child() this was solved with commit 236286. For free_child() this was solved together with some other changes in STABLE 10 and 11 but reverted because of another problem reported in PR 230196. 

For Stable 11 the following patch would be enough to close this PR:

--- popen.c.orig        2019-02-18 19:07:59.043267000 +0100
+++ popen.c     2019-02-18 19:09:25.226232000 +0100
@@ -359,12 +359,14 @@
 free_child(int pid)
 {
        sigset_t nset, oset;
-       struct child *cp = findchild(pid);
+       struct child *cp;

        (void)sigemptyset(&nset);
        (void)sigaddset(&nset, SIGCHLD);
        (void)sigprocmask(SIG_BLOCK, &nset, &oset);

+       cp = findchild(pid);
+
        if (cp->done)
                delchild(cp);
        else