Project

General

Profile

Actions

Bug #14208

closed

zoneadmd should not inherit zoneadm's environment

Added by Andy Fiddaman 7 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
zones
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

The zoneadmd daemon for a zone is started on-demand by zoneadm. For zones which are set to auto-boot, this occurs via the system/zones service, running under svc.startd, and otherwise it is generally an interactive shell.

zoneadmd ends up with the same environment as that zoneadm, including the path, and passes this to children such as the boot, pre- and post-state helpers, and it should not.

An example of a zone which was started by system/zones:

reaper# penv 773
773:    zoneadmd -z bloody
...
envp[30]: _ZONEADM_LOCK_HELD=1
envp[31]: _=*741*/usr/sbin/zoneadm
envp[32]: PATH=/usr/sbin:/usr/bin
envp[33]: PWD=/
envp[34]: SHLVL=2
envp[35]: SMF_FMRI=svc:/system/zones:default
envp[36]: SMF_METHOD=start
envp[37]: SMF_RESTARTER=svc:/system/svc/restarter:default
envp[38]: SMF_ZONENAME=global
envp[39]: TZ=UTC
envp[40]: _AST_FEATURES=UNIVERSE - att
envp[41]: A__z="*SHLVL

and one started later from a shell:

reaper# penv 7900
7900:   zoneadmd -z build
...
envp[35]: _ZONEADM_LOCK_HELD=1
envp[36]: PERLDOC_PAGER=/usr/bin/less -rins
envp[37]: STY=2416.pts-1.reaper
envp[38]: TERM=screen-256color
envp[39]: TERMCAP=SC|screen-256color|VT 100/ANSI X3.64 virtual terminal:DO=\\E[%dB:LE=\\E[%dD:RI=\\E[%dC:UP=\\E[%dA:bs:bt=\\E[Z:cd=\\E[J:ce=\\E[K:cl=\\E[H\\E[J:cm=\\E[%i%d;%dH:ct=\\E[3g:do=^J:nd=\\E[C:pt:rc=\\E8:rs=\\Ec:sc=\\E7:st=\\EH:up=\\EM:le=^H:bl=^G:cr=^M:it#8:ho=\\E[H:nw=\\EE:ta=^I:is=\\E)0:li#45:co#80:am:xn:xv:LP:sr=\\EM:al=\\E[L:AL=\\E[%dL:cs=\\E[%i%d;%dr:dl=\\E[M:DL=\\E[%dM:im=\\E[4h:ei=\\E[4l:mi:IC=\\E[%d@:ks=\\E[?1h\\E=:ke=\\E[?1l\\E>:vi=\\E[?25l:ve=\\E[34h\\E[?25h:vs=\\E[34l:ti=\\E[?1049h:te=\\E[?1049l:us=\\E[4m:ue=\\E[24m:so=\\E[3m:se=\\E[23m:mb=\\E[5m:md=\\E[1m:mr=\\E[7m:me=\\E[m:ms:Co#8:pa#64:AF=\\E[3%dm:AB=\\E[4%dm:op=\\E[39;49m:AX:vb=\\Eg:G0:as=\\E(0:ae=\\E(B:ac=\\140\\140aaffggjjkkllmmnnooppqqrrssttuuvvwwxxyyzz{{||}}~~..--++,,hhII00:Km=\\E[M:k0=\\E[10~:k1=\\EOP:k2=\\EOQ:k3=\\EOR:k4=\\EOS:k5=\\E[15~:k6=\\E[17~:k7=\\E[18~:k8=\\E[19~:k9=\\E[20~:k;=\\E[21~:F1=\\E[23~:F2=\\E[24~:kB=\\E[Z:kh=\\E[1~:@1=\\E[1~:kH=\\E[4~:@7=\\E[4~:kN=\\E[6~:kP=\\E[5~:kI=\\E[2~:kD=\\E[3~:ku=\\EOA:kd=\\EOB:kr=\\EOC:kl=\\EOD:
envp[40]: WINDOW=4
envp[41]: SHELL=/usr/bin/zsh
envp[42]: _=/opt/ooce/bin/zadm
envp[43]: CC=gcc
envp[44]: LESS=-iEMnQX
envp[45]: OLDPWD=/home/af
envp[46]: PWD=/home/af
envp[47]: SHLVL=3
envp[48]: USER=af
envp[49]: LOGNAME=af
envp[50]: HOME=/home/af
envp[51]: PATH=/home/af/bin:/usr/bin:/usr/sbin:/opt/ooce/bin:/opt/bin
envp[52]: TZ=UTC
envp[53]: SSH_CLIENT=x.x.x.x 56055 22
envp[54]: SSH_CONNECTION=x.x.x.x 56055 x.x.x.x 22
envp[55]: SSH_TTY=/dev/pts/1

Related issues

Related to illumos gate - Bug #14406: os-tests: definit test entry incorrectClosedRobert Mustacchi

Actions
Actions #1

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1776
Actions #2

Updated by Till Wegmüller 7 months ago

We will need to test this with OI's IPS as this has impact one it.

Actions #3

Updated by Andy Fiddaman 7 months ago

This change has been re-worked.

To always start zoneadmd with a consistent environment, zoneadm now builds an environment containing PATH, the zoneadm lock variable and the values taken from /etc/default/init.

Since the code to parse /etc/default/init was already duplicated in init and svc.startd, I centralised this by introducing a new iterator under usr/src/common/definit and updated init, svc.startd and libzonecfg to use this.

I've tested this in various ways:

  • I ran the new tests that accompany this change;
  • I booted an unmodified system having replaced /etc/default/init with the one from the testsuite that accompanies this change and captured the environment of svc.startd (started by init), dlmgmtd (started by svc.startd).
    I then booted a system with this change and the same environment file and compared the output. The only difference was due to the old code not allowing ; to be embedded within single quotes, which appears to be a bug in the old code:
build% diff -u {baseline,new}.dlmgmtd
--- baseline.dlmgmtd    Wed Nov  3 12:06:34 2021
+++ new.dlmgmtd Wed Nov  3 12:14:46 2021
@@ -1,5 +1,5 @@
-37:    /sbin/dlmgmtd
-envp[0]: _=*35*/sbin/dlmgmtd
+100033:        /sbin/dlmgmtd
+envp[0]: _=*100031*/sbin/dlmgmtd
 envp[1]: BAD=54
 envp[2]: BARNEY=dino
 envp[3]: BOB=fred
@@ -28,8 +28,8 @@
 envp[26]: PATH=/usr/sbin:/usr/bin
 envp[27]: PWD=/
 envp[28]: QSC=test1;tryst2
-envp[29]: QSCS=test3
-envp[30]: QSCSS=test5
+envp[29]: QSCS=test3;tryst4
+envp[30]: QSCSS=test5;tryst6
 envp[31]: QSCSSS=test5;tryst6
 envp[32]: SHLVL=2
 envp[33]: SMF_FMRI=svc:/network/datalink-management:default

The test data for the portion that the diff covers is:

QSC="test1;tryst2" 
QSCS='test3;tryst4'
QSCSS='test5;tryst6" 
QSCSSS="test5;tryst6'

and the old code just lost the ; and the token afterwards when single quotes were used.
Note that the header of /etc/default/init states:

value may be enclosed in double quotes (") or single quotes (').

I have checked the environment of a zone booted automatically (via SMF) and then one started from a shell with a deliberately modified environment, and confirmed that the environment is properly set in both cases. Unlike previously, a command such as:

% umask 000
% TZ=UTC LC_ALL=C PATH=/dev/null /usr/bin/pfexec /usr/sbin/zoneadm -z <zone> boot

will no-longer cause the zone boot to fail, and I have verified that the umask is properly reset for the new zoneadmd process, as well as the environment.

bloody# cat /etc/default/init
#
# This file is /etc/default/init.
# This file looks like a shell script, but it is not.
#
# Lines of this file should be of the form VAR=value, where VAR is one of
# TZ, LANG, CMASK, or any of the LC_* environment variables.  value may
# be enclosed in double quotes (") or single quotes (').
#
#TZ=UTC
TZ=Iran
CMASK=022
    UMEM_DEBUG="default"; BOB=fred; SUE='test'
#LC_ALL=C
  LC_ALL=en_GB.UTF-8
           #indented comment

bloody# sed -i '/CMASK/s/022/033/' /etc/default/init
bloody# rg CMASK /etc/default/init
11:CMASK=033
bloody# pgrep zoneadmd
bloody# truss -flea -t umask zoneadm -z lxkeep boot
101060/1:       execve("/usr/sbin/zoneadm", 0x0803D17C, 0x0803D190)  argc = 4
101060/1:        argv: zoneadm -z lxkeep boot
101060/1:        envp: _=/usr/bin/truss omniosorg=/data/omnios-build/omniosorg
101060/1:         OLDPWD=/data/omnios-build PWD=/data/omnios-build SHLVL=2
101060/1:         LC_ALL=en_GB.UTF-8 USER=af LOGNAME=af HOME=/data/omnios-build
101060/1:         PATH=/usr/bin:/usr/sbin:/sbin:/usr/gnu/bin:/opt/ooce/bin/opt/onbld/bin/i386:/opt/onbld/bin:/data/omnios-build/bin:/opt/ooce/bin:/opt/bin/opt/onbld/bin/i386:/opt/onbld/bin:/data/omnios-build/bin:/opt/ooce/bin:/opt/bin
101060/1:         SHELL=/bin/zsh TZ=Iran TERM=screen-256color
101060/1:         SSH_CLIENT=172.27.10.79 33541 22
101060/1:         SSH_CONNECTION=172.27.10.79 33541 172.27.10.9 22
101060/1:         SSH_TTY=/dev/pts/1
101061/1:       umask(033)                                      = 022
101061/1:       execve("/usr/lib/zones/zoneadmd", 0x0803C1D4, 0x08C8EAC8)  argc = 3
101061/1:        argv: zoneadmd -z lxkeep
101061/1:        envp: _ZONEADM_LOCK_HELD=1 PATH=/usr/sbin:/usr/bin TZ=Iran
101061/1:         UMEM_DEBUG=default BOB=fred SUE=test LC_ALL=en_GB.UTF-8
Actions #4

Updated by Electric Monk 6 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit a28480febf31f0e61debac062a55216a98a05a92

commit  a28480febf31f0e61debac062a55216a98a05a92
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2021-11-23T18:12:15.000Z

    14208 zoneadmd should not inherit zoneadm's environment
    Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
    Reviewed by: Gergő Mihály Doma <domag02@gmail.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Reviewed by: Jason King <jason.brian.king@gmail.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions #5

Updated by Robert Mustacchi 4 months ago

  • Related to Bug #14406: os-tests: definit test entry incorrect added
Actions

Also available in: Atom PDF