NetBSD Problem Report #46546

From fukumoto@imasy.or.jp  Tue Jun  5 16:20:37 2012
Return-Path: <fukumoto@imasy.or.jp>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id 89A4263BA27
	for <gnats-bugs@gnats.netbsd.org>; Tue,  5 Jun 2012 16:20:37 +0000 (UTC)
Message-Id: <20120605160213.116CF71@kestrel.wings.imasy.or.jp>
Date: Wed,  6 Jun 2012 01:02:13 +0900 (JST)
From: fukumoto@imasy.or.jp
Reply-To: fukumoto@imasy.or.jp
To: gnats-bugs@gnats.NetBSD.org
Subject: powerd script fails due to spurious environment variable
X-Send-Pr-Version: 3.95

>Number:         46546
>Category:       misc
>Synopsis:       powerd script fails due to environment variable passed from rc
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    misc-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 05 16:25:00 +0000 2012
>Closed-Date:    Sun Jul 27 07:48:13 +0000 2014
>Last-Modified:  Sun Jul 27 07:50:01 +0000 2014
>Originator:     fukumoto@imasy.or.jp
>Release:        NetBSD 6.0_BETA2
>Organization:

>Environment:


System: NetBSD kestrel 6.0_BETA2 NetBSD 6.0_BETA2 (GENERIC) #7: Wed May 23 23:37:40 JST 2012 fukumoto@kestrel:/u/src/sys/arch/amd64/compile/obj/GENERIC amd64
Architecture: x86_64
Machine: amd64
>Description:
	As I reported in http://mail-index.netbsd.org/netbsd-users/2012/06/03/msg010808.html
	powerd(8) script fails due to the environment variable passed from rc(8).

	When booting the system, rc_real_work() in /etc/rc defines the
	environment variable $_rc_postprocessor_fd, and spawns daemons
	including powerd(8).  Later, when sleep button is pressed,
	powerd(8) calls /etc/powerd/scripts/sleep_button, which then
	calls /etc/rc.d/network.  It includes /etc/rc.subr, which
	overrides the normal "echo" with its own echo() shell
	function.  Thus "echo -n" inside network script calls
	_flush_rc_output(), which calls print_rc_metadata(), which,
	since $_rc_postprocessor_fd is defined, tries to transmit a
	metadata to postprocessor through $_rc_postprocessor_fd, but
	the postprocessor doesn't exist anymore when powerd script is
	executed, and the network script fails for SIGPIPE.

>How-To-Repeat:
	Set powerd=YES in /etc/rc.conf and restart the system.  Press
	the sleep button to sleep, then resume the system and observe
	/etc/rc.d/network exits midway.

>Fix:
	Either run_rc_command or powerd should clean-up the
	environment variables before spawning subprocesses.

>Release-Note:

>Audit-Trail:
From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: misc/46546: powerd script fails due to spurious environment
 variable
Date: Fri, 25 Jul 2014 18:39:35 +0200

 --bAmEntskrkuBymla
 Content-Type: text/plain; charset=us-ascii; format=flowed
 Content-Disposition: inline

 Please try the attached patch, which attempts to notice when a process that 
 was started under control of /etc/rc no longer has access to /etc/rc's output 
 postprocessor (which will be the case for daemons launched from rc.d scripts).

 --apb (Alan Barrett)

 --bAmEntskrkuBymla
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="pr46546.patch"

 Index: etc/rc
 ===================================================================
 --- etc/rc	9 Apr 2014 12:45:05 -0000	1.168
 +++ etc/rc	25 Jul 2014 12:35:33 -0000
 @@ -84,6 +84,7 @@ rc_real_work()
  	# with redirected output.
  	#
  	_rc_postprocessor_fd=9 ; export _rc_postprocessor_fd
 +	_rc_pid=$$ ; export _rc_pid
  	eval "exec ${_rc_postprocessor_fd}>&1"

  	# Print a metadata line when we exit
 Index: etc/rc.subr
 ===================================================================
 --- etc/rc.subr	17 Dec 2012 18:17:27 -0000	1.92
 +++ etc/rc.subr	25 Jul 2014 14:45:13 -0000
 @@ -786,6 +786,35 @@ $command $rc_flags $command_args"
  }

  #
 +# _have_rc_postprocessor
 +#	Test whether the current script is running in a context that
 +#	was invoked from /etc/rc with a postprocessor.
 +#
 +#	If the test fails, some variables may be unset to make
 +#	such tests more efficient in future.
 +#
 +_have_rc_postprocessor()
 +{
 +	# Cheap tests that fd and pid are set, fd is writable.
 +	[ -n "${_rc_postprocessor_fd}" ] || return 1
 +	[ -n "${_rc_pid}" ] || return 1
 +	eval ": >&${_rc_postprocessor_fd}" 2>/dev/null || return 1
 +
 +	# More expensive test that pid is running.
 +	# Unset _rc_pid if this fails.
 +	kill -0 "${_rc_pid}" 2>/dev/null \
 +	|| { unset _rc_pid; return 1; }
 +
 +	# More expensive test that pid appears to be
 +	# a shell running an rc script.
 +	# Unset _rc_pid if this fails.
 +	expr "$(ps -p "${_rc_pid}" -o command=)" : ".*sh .*/rc.*" >/dev/null \
 +	|| { unset _rc_pid; return 1; }
 +
 +	return 0
 +}
 +
 +#
  # run_rc_script file arg
  #	Start the script `file' with `arg', and correctly handle the
  #	return value from the script.  If `file' ends with `.sh', it's
 @@ -794,9 +823,8 @@ $command $rc_flags $command_args"
  #	executable run as a child process.
  #
  #	If `file' contains "KEYWORD: interactive" and if we are
 -#	running inside /etc/rc with postprocessing (as signified by
 -#	_rc_postprocessor_fd being defined) then the script's stdout
 -#	and stderr are redirected to $_rc_original_stdout_fd and
 +#	running inside /etc/rc with postprocessing, then the script's
 +#	stdout and stderr are redirected to $_rc_original_stdout_fd and
  #	$_rc_original_stderr_fd, so the output will be displayed on the
  #	console but not intercepted by /etc/rc's postprocessor.
  #
 @@ -816,7 +844,7 @@ run_rc_script()
  	eval unset ${_arg}_cmd ${_arg}_precmd ${_arg}_postcmd

  	_must_redirect=false
 -	if [ -n "${_rc_postprocessor_fd}" ] \
 +	if _have_rc_postprocessor \
  	    && _has_rcorder_keyword interactive $_file
  	then
  		_must_redirect=true
 @@ -1114,7 +1142,7 @@ print_rc_metadata()
  	# _rc_postprocessor fd, if defined, is the fd to which we must
  	# print, prefixing the output with $_rc_metadata_prefix.
  	#
 -	if [ -n "$_rc_postprocessor_fd" ]; then
 +	if _have_rc_postprocessor; then
  		command printf "%s%s\n" "$rc_metadata_prefix" "$1" \
  			>&${_rc_postprocessor_fd}
  	fi
 @@ -1154,10 +1182,11 @@ _flush_rc_output()
  #
  print_rc_normal()
  {
 -	# If _rc_postprocessor_fd is defined, then it is the fd
 -	# to which we must print; otherwise print to stdout.
 +	# print to stdout or _rc_postprocessor_fd, depending on
 +	# whether not we have an rc postprocessor.
  	#
 -	local fd="${_rc_postprocessor_fd:-1}"
 +	local fd=1
 +	_have_rc_postprocessor && fd="${_rc_postprocessor_fd}"
  	case "$1" in
  	"-n")
  		command printf "%s" "$2" >&${fd}
 @@ -1186,7 +1215,7 @@ print_rc_normal()
  #
  no_rc_postprocess()
  {
 -	if [ -n "${_rc_postprocessor_fd}" ]; then
 +	if _have_rc_postprocessor; then
  		"$@" >&${_rc_original_stdout_fd} 2>&${_rc_original_stderr_fd}
  	else
  		"$@"

 --bAmEntskrkuBymla--

From: fukumoto@imasy.or.jp
To: gnats-bugs@NetBSD.org, apb@cequrux.com
Cc: misc-bug-people@netbsd.org, gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org
Subject: Re: misc/46546: powerd script fails due to spurious environment
 variable
Date: Sun, 27 Jul 2014 09:35:20 +0900 (JST)

 Cofirmed that the patch remedies the problem.  Thank you.


 					FUKUMOTO Atsushi
 					fukumoto@imasy.or.jp

From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: misc/46546: powerd script fails due to spurious environment
 variable
Date: Sun, 27 Jul 2014 09:33:38 +0200

 On Sun, 27 Jul 2014, fukumoto@imasy.or.jp wrote:
 > Cofirmed that the patch remedies the problem.  Thank you.

 Thank you for the initial report, and for testing.  I am sorry 
 that it took so long to fix, because I didn't notice the original 
 report until a few days ago.

 --apb (Alan Barrett)

State-Changed-From-To: open->closed
State-Changed-By: apb@NetBSD.org
State-Changed-When: Sun, 27 Jul 2014 07:48:13 +0000
State-Changed-Why:
fixed


From: "Alan Barrett" <apb@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46546 CVS commit: src/etc
Date: Sun, 27 Jul 2014 07:46:46 +0000

 Module Name:	src
 Committed By:	apb
 Date:		Sun Jul 27 07:46:46 UTC 2014

 Modified Files:
 	src/etc: rc rc.subr

 Log Message:
 In rc.subr, add _have_rc_postprocessor function and use it instead of
 inline tests like [ -n "${_rc_postprocessor_fd}" ]. The new function
 performs a few new tests, including verifying that /etc/rc is still
 running (using a new _rc_pid variable set by /etc/rc).

 This is intended to deal with the case that a script run from /etc/rc
 spawns a background process, then /etc/rc exits, but the background
 process still has environment variables inherited from /etc/rc.

 Fixes PR 46546.


 To generate a diff of this commit:
 cvs rdiff -u -r1.168 -r1.169 src/etc/rc
 cvs rdiff -u -r1.93 -r1.94 src/etc/rc.subr

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

>Unformatted:

Home	PR Database Search