NetBSD Problem Report #35366

From joe@localhost.joe.cz  Sat Jan  6 11:47:29 2007
Return-Path: <joe@localhost.joe.cz>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id C0DD363B880
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  6 Jan 2007 11:47:29 +0000 (UTC)
Message-Id: <200701061046.l06Akwm7009102@localhost.>
Date: Sat, 6 Jan 2007 11:46:58 +0100 (CET)
From: Dominik Joe Pantucek <joe@joe.cz>
Reply-To: Dominik Joe Pantucek <joe@joe.cz>
To: gnats-bugs@NetBSD.org
Subject: /etc/rc.subr _find_processes doesn't always find existing process when it is interpreted program.
X-Send-Pr-Version: 3.95

>Number:         35366
>Category:       misc
>Synopsis:       /etc/rc.subr _find_processes doesn't always find existing process when it is interpreted program.
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    misc-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Jan 06 11:50:00 +0000 2007
>Closed-Date:    Sat Jan 27 14:31:05 +0000 2007
>Last-Modified:  Sat Feb 10 14:20:01 +0000 2007
>Originator:     Dominik Joe Pantucek <joe@joe.cz>
>Release:        NetBSD 3.0.1
>Organization:

>Environment:
System: NetBSD Osgiliath 3.0.1 NetBSD 3.0.1 (OSGILIATH) #2: Wed Oct 11 15:47:51 CEST 2006 root@Osgiliath:/usr/obj/sys/arch/i386/compile/OSGILIATH i386
Architecture: i386
Machine: i386
>Description:
	Found when using "/etc/rc.d/amavisd status" command with the default rc.d script from amavisd pkgsrc distribution. It always shows amavisd as not running. The reason for this is bad match in _find_processes function in /etc/rc.subr. The 1.65 version (netbsd-3.0.1) doesn't handle such situations at all and 1.67 (-current) handles them in slightly insufficient way.
	The problem is exposed, when programs alter their argv[] from full paths to just basenames - like /usr/bin/perl -> perl, /usr/pkg/sbin/amavisd -> amavisd.
	1.65 - doesn't handle this at all
	1.67 - handles interpreter name change, not the script name change (it can handle "perl /usr/pkg/sbin/amavisd" not "perl: amavis")
>How-To-Repeat:
	Install amavisd or other package from pkgsrc which has rc.d script that uses rc.subr and its daemon is interpreted script modifying argv.
	Copy its rc.d script to /etc/rc.d (or run it from other location like /usr/pkg/etc/rc.d)
	Run /etc/rc.d/amavisd start (or equivalent)
	Run /etc/rc.d/amavisd status (dtto)
	It will tell you the daemon is not running.
>Fix:
	I've created two patches which fix this in a clean way. One for 1.65 and other for 1.67 version of rc.subr.
	http://joe.cz/netbsd/rc.subr-1.65-shortmatch.diff
	http://joe.cz/netbsd/rc.subr-1.67-shortmatch.diff
	The latter removes the insufficient match from 1.67 and replaces it with fully abbreviated match (see source).

>Release-Note:

>Audit-Trail:
From: Hubert Feyrer <hubert@feyrer.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: misc/35366
Date: Fri, 19 Jan 2007 13:20:27 +0100 (CET)

 2nd try to get this into gnats... - HF

 ---------- Forwarded message ----------
 Date: Fri, 19 Jan 2007 01:10:27 +0100 (CET)
 From: Hubert Feyrer <hubert@feyrer.de>
 To: Dominik Joe Pantucek <joe@joe.cz>
 Cc: gnats-bugs@NetBSD.org, he@NetBSD.org, Hubert Feyrer <hubert@feyrer.de>
 Subject: Re: PR misc/35366


 [CC:ing he@ for rev. 1.66 of rc.subr & PR 31932]


 Hi,

 following up our chat on IRC tonight, I'm sending you this mail to document the 
 latest state of things, also in the PR (hopefully...).

 Citing you from IRC:

 <dzoe> But it is the same as the cas 1.66 addresses
 <dzoe> But not for interpreter, but for the interpreted program itselft.
 <dzoe>  /path/to/interp /path/to/program
 <dzoe>  /path/to/interp program
 <dzoe>  interp /path/to/program
 <dzoe>  interp program
 <dzoe> 1.66 handles first 3 cases
 <dzoe> Why not the fourth, especially when the 4th is only a variation on 3rd.
 <dzoe> 1.66 catches the "perl: bla" case as long as bla=/path/to/program

 rev. 1.66 was in response to PR 31932, it documents postgrey running as

     476 ?     IWs   0:41.25 perl: /usr/pkg/sbin/postgrey -i 2525 -d 
 --pidfile=/var/run/postgrey.pid (perl)

 while for amavis this is something like:

   16541 ?     S<s    0:06.44 perl: amavisd (master)

 so the difference is to either match a full or short pathname in the _proccheck 
 variable, after the interpreter and the ": ". The patch below is also available 
 from http://www.feyrer.de/Misc/pr-35366-patch4, please test and let me know if 
 it improves the situation for you.


   - Hubert


 --- /usr/cvs/src-current/etc/rc.subr	2007-01-18 22:24:25.000000000 +0100
 +++ /etc/rc.subr	2007-01-19 01:07:44.000000000 +0100
 @@ -179,6 +179,7 @@
   	_psargs=$3

   	_pref=
 +	_procnamebn=${_procname##*/}
   	if [ $_interpreter != "." ]; then	# an interpreted script
   		read _interp < ${_chroot:-}/$_procname	# read interpreter name
   		_interp=${_interp#\#!}		# strip #!
 @@ -190,9 +191,8 @@
   		_interpbn=${1##*/}
   		_fp_args='_argv'
   		_fp_match='case "$_argv" in
 -		    ${_interp}|"${_interp} "*|"${_interpbn}: ${_procname}"*)'
 +		    ${_interp}|"${_interp} "*|"${_interpbn}: 
 "*${_procnamebn}*)'
   	else					# a normal daemon
 -		_procnamebn=${_procname##*/}
   		_fp_args='_arg0 _argv'
   		_fp_match='case "$_arg0" in
   		    $_procname|$_procnamebn|${_procnamebn}:|"(${_procnamebn})")'

From: Hubert Feyrer <hubert@feyrer.de>
To: he@NetBSD.org
Cc: "Dominik Joe Pantu*c(ek" <joe@joe.cz>, gnats-bugs@NetBSD.org
Subject: Re: misc/35366
Date: Sat, 20 Jan 2007 02:35:51 +0100 (CET)

 Havard,

 can you please sanity-check the patch I mailed, before I commit it?


   - Hubert

 On Fri, 19 Jan 2007, Dominik Joe Pantu*c(ek wrote:

 > Hello,
 >
 >   it works and handles the actual amavisd case correctly. The only case
 > it doesn't handle is the test case I showed you on irc. If the daemon
 > forks itself without adding ':' between interpreter and script. I don't
 > know of any real program that behaves this way, so it might not be
 > neccessary. If this is true, then your patch would be sufficient.
 >
 > Joe
 > Hubert Feyrer napsal(a):
 >>
 >> [CC:ing he@ for rev. 1.66 of rc.subr & PR 31932]
 >>
 >>
 >> Hi,
 >>
 >> following up our chat on IRC tonight, I'm sending you this mail to
 >> document the latest state of things, also in the PR (hopefully...).
 >>
 >> Citing you from IRC:
 >>
 >> <dzoe> But it is the same as the cas 1.66 addresses
 >> <dzoe> But not for interpreter, but for the interpreted program itselft.
 >> <dzoe>  /path/to/interp /path/to/program
 >> <dzoe>  /path/to/interp program
 >> <dzoe>  interp /path/to/program
 >> <dzoe>  interp program
 >> <dzoe> 1.66 handles first 3 cases
 >> <dzoe> Why not the fourth, especially when the 4th is only a variation
 >> on 3rd.
 >> <dzoe> 1.66 catches the "perl: bla" case as long as bla=/path/to/program
 >>
 >> rev. 1.66 was in response to PR 31932, it documents postgrey running as
 >>
 >>    476 ?     IWs   0:41.25 perl: /usr/pkg/sbin/postgrey -i 2525 -d
 >> --pidfile=/var/run/postgrey.pid (perl)
 >>
 >> while for amavis this is something like:
 >>
 >>  16541 ?     S<s    0:06.44 perl: amavisd (master)
 >>
 >> so the difference is to either match a full or short pathname in the
 >> _proccheck variable, after the interpreter and the ": ". The patch
 >> below is also available from
 >> http://www.feyrer.de/Misc/pr-35366-patch4, please test and let me know
 >> if it improves the situation for you.
 >>
 >>
 >>  - Hubert
 >>
 >>
 >> --- /usr/cvs/src-current/etc/rc.subr    2007-01-18 22:24:25.000000000
 >> +0100
 >> +++ /etc/rc.subr    2007-01-19 01:07:44.000000000 +0100
 >> @@ -179,6 +179,7 @@
 >>      _psargs=$3
 >>
 >>      _pref=
 >> +    _procnamebn=${_procname##*/}
 >>      if [ $_interpreter != "." ]; then    # an interpreted script
 >>          read _interp < ${_chroot:-}/$_procname    # read interpreter
 >> name
 >>          _interp=${_interp#\#!}        # strip #!
 >> @@ -190,9 +191,8 @@
 >>          _interpbn=${1##*/}
 >>          _fp_args='_argv'
 >>          _fp_match='case "$_argv" in
 >> -            ${_interp}|"${_interp} "*|"${_interpbn}: ${_procname}"*)'
 >> +            ${_interp}|"${_interp} "*|"${_interpbn}: "*${_procnamebn}*)'
 >>      else                    # a normal daemon
 >> -        _procnamebn=${_procname##*/}
 >>          _fp_args='_arg0 _argv'
 >>          _fp_match='case "$_arg0" in
 >>              $_procname|$_procnamebn|${_procnamebn}:|"(${_procnamebn})")'
 >
 >
 >

State-Changed-From-To: open->closed
State-Changed-By: hubertf@netbsd.org
State-Changed-When: Sat, 27 Jan 2007 14:31:05 +0000
State-Changed-Why:
I've committed the fix, and I'll wait a few more days before sending
a pullup request for netbsd-4.
Thanks!


 - Hubert


From: Hubert Feyrer <hubertf@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: PR/35366 CVS commit: src/etc
Date: Sat, 27 Jan 2007 14:30:27 +0000 (UTC)

 Module Name:	src
 Committed By:	hubertf
 Date:		Sat Jan 27 14:30:27 UTC 2007

 Modified Files:
 	src/etc: rc.subr

 Log Message:
 _find_processes(): in addition to the existing "interpreter: /path/to/daemon"
 also recognize "interpreter: daemon" in ps(1) output.

 That case statement should be rewritten with expr(1) if more flexibility is
 needed.

 Fixes PR 35366 by Dominik Joe Pantucek, debugged with much assistance
 by him on IRC.


 To generate a diff of this commit:
 cvs rdiff -r1.67 -r1.68 src/etc/rc.subr

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

From: Matthias Scheler <tron@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: PR/35366 CVS commit: [netbsd-4] src/etc
Date: Sat, 10 Feb 2007 14:17:21 +0000 (UTC)

 Module Name:	src
 Committed By:	tron
 Date:		Sat Feb 10 14:17:21 UTC 2007

 Modified Files:
 	src/etc [netbsd-4]: rc.subr

 Log Message:
 Pull up following revision(s) (requested by hubertf in ticket #433):
 	etc/rc.subr: revision 1.68
 _find_processes(): in addition to the existing "interpreter: /path/to/daemon"
 also recognize "interpreter: daemon" in ps(1) output.
 That case statement should be rewritten with expr(1) if more flexibility is
 needed.
 Fixes PR 35366 by Dominik Joe Pantucek, debugged with much assistance
 by him on IRC.


 To generate a diff of this commit:
 cvs rdiff -r1.67 -r1.67.2.1 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