Bug#802018: systemd: _SYSTEMCTL_SKIP_REDIRECT=true is leaked to daemon

2016-12-25 Thread Martin Pitt
Hello all,

Michael Biebl [2016-12-22  9:21 +0100]: > Am 16.10.2015 um 23:07 schrieb 
Sebastian Schmidt:
> Looking at that old code again, I actually don't remember, why we export
>  _SYSTEMCTL_SKIP_REDIRECT="true" at all and if the obvious fix would be
> to simply drop that else clause.

AFAIK the original intent was that if you call "/etc/init.d/foo start" this
would redirect to "systemctl start" which would run "Exec=/etc/init.d/foo
start" in the generated unit which must not do that same redirection again.

This should be sufficiently covered by the [ $PPID -ne 1 ] test, though. git
archeology shows that commit c04d0f71 introduced the script and both the
SKIP_REDIRECT and the [ $PPID -ne 1 ] were already present, though.

So, I can't think of a good reason to set this either, thanks for dropping it
from master.

Martin
-- 
Martin Pitt| http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)


signature.asc
Description: PGP signature


Bug#802018: systemd: _SYSTEMCTL_SKIP_REDIRECT=true is leaked to daemon

2016-12-22 Thread Michael Biebl
Am 16.10.2015 um 23:07 schrieb Sebastian Schmidt:
> Package: systemd
> Version: 226-4
> Severity: normal
> Tags: 
> 
> Hi,
> 
> /lib/lsb/init-functions.d/40-systemd exports
> _SYSTEMCTL_SKIP_REDIRECT=true to detect when it has been called
> recursively again by systemd. This export is actually never cleaned up and
> is passed to the daemon. In the case of Puppet, which uses “/etc/init.d
> $service $action” (without cleaning up the environment - which is
> probably an issue by itself) to query and change a daemon’s status, this
> causes the init.d hook to not redirect the call to systemd. For services
> that supply an actual systemd unit which does not simply call the init
> script (e.g. clamav-freshclam) this can cause the action to fail.
> 
> A trivial fix would be:
>>   --- a/debian/extra/init-functions.d/40-systemd
>>   +++ b/debian/extra/init-functions.d/40-systemd
>>   @@ -25,6 +25,8 @@ if [ -d /run/systemd/system ]; then
>>fi
>>;;
>>esac
>>   +elif [ -z "${_SYSTEMCTL_SKIP_REDIRECT:-}" ]; then
>>   +unset _SYSTEMCTL_SKIP_REDIRECT
>>else
>>export _SYSTEMCTL_SKIP_REDIRECT="true"
>>fi
> 


Looking at that old code again, I actually don't remember, why we export
 _SYSTEMCTL_SKIP_REDIRECT="true" at all and if the obvious fix would be
to simply drop that else clause.

I did that and didn't encounter any obvious problems.
As said, this code is old, over 6 years, so I don't remember. Can anyone
think of a scenario where we would need that?

Andeas mentioned on IRC that it could be to avoid redirect loops, but
how would this work. As soon as we redirect the call to systemctl, the
actual execution will be by systemd, i.e. PID 1 and PPID will be 1.

Can there be a case where this assumption is not correct?

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?

-- 
Why is it that all of the instruments seeking intelligent life in the
universe are pointed away from Earth?



signature.asc
Description: OpenPGP digital signature


Bug#802018: systemd: _SYSTEMCTL_SKIP_REDIRECT=true is leaked to daemon

2015-10-16 Thread Sebastian Schmidt
Package: systemd
Version: 226-4
Severity: normal
Tags: 

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi,

/lib/lsb/init-functions.d/40-systemd exports
_SYSTEMCTL_SKIP_REDIRECT=true to detect when it has been called
recursively again by systemd. This export is actually never cleaned up and
is passed to the daemon. In the case of Puppet, which uses “/etc/init.d
$service $action” (without cleaning up the environment - which is
probably an issue by itself) to query and change a daemon’s status, this
causes the init.d hook to not redirect the call to systemd. For services
that supply an actual systemd unit which does not simply call the init
script (e.g. clamav-freshclam) this can cause the action to fail.

A trivial fix would be:
>   --- a/debian/extra/init-functions.d/40-systemd
>   +++ b/debian/extra/init-functions.d/40-systemd
>   @@ -25,6 +25,8 @@ if [ -d /run/systemd/system ]; then
>fi
>;;
>esac
>   +elif [ -z "${_SYSTEMCTL_SKIP_REDIRECT:-}" ]; then
>   +unset _SYSTEMCTL_SKIP_REDIRECT
>else
>export _SYSTEMCTL_SKIP_REDIRECT="true"
>fi

But this doesn’t fix the case when systemd is not calling the init
script twice (because the service has an actual unit). Since the
approach to guard the recursion with an environment variable relies on
systemd not cleaning up the environment I’m not sure how to best fix
this.

Sebastian

-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIVAwUBViFm/Phx3EthBlqjAQgIQhAAowyPvUlhWh1wyDGotGFXOQGyHwFx5ov4
afMErBsvfnySQlG56vXZNzhVEqg+Ta7jL+oxlVemZ2PIGowxnp4blV4D5k3VZy63
8bBnwmfnNTVYPG3c3b5tXEADerLz5vZ6qVv/V5OM1KQcVS4hhlxVLFDkZTv9sh3T
Lu/0v7sL1M/t2EdN0GRHYex6Pp7hwZajG8ZmHIUfrgj2yvkAxv6ObRTKLHy1Cjzq
Pc8gKbDtFctoYz6Z4BjfuwTZ8yGVuE85j7lAENCFDczB6w7T3GFiAyWzgg/lGk2a
atA0sZ541nmll4dnZZCoPPn2YVtteWeLKneXtT6q8gyEaIuBHr860EXxh5V1RjFr
gAJIx0fJnk/Y6bT4aYrhEHI3o88EZ+D8pdNyzLvGZt5aeWI0rotakpnBbfXHuPae
Q5IHFiuWx4gV3QP7RBk5+OXQXPhXjs/ntDTrlioIzkAFHBO3f9l75LdimcJUJwIl
dwmhiKYQsah0EfTY2T+gmWclgGsxouUG6cACbx9xc97EkhIy1wxFR1O3/lvnH89F
a5IQp0AMS+1JgjloBXxPFI513Xi3H0w+4H4oC+4ET4vKMqMeS4x8RC3EKaxTe1ep
mOuvQYgCEOVHZuTrGytApHNOnlKhu7ZRAeoxtrDcub2EgJNcVu/0h2DjvgfLwW+W
iGjTUHU3l6w=
=EsOG
-END PGP SIGNATURE-