Bug 5359

Summary: [Yocto 1.5] su - USER -c 'COMMAND' env variable issue
Product: [Runtime] General Runtime Reporter: Davide Soldan <davide.soldan>
Component: General RuntimeAssignee: Chen Qi <Qi.Chen>
Status: RESOLVED FIXED QA Contact: Bogdan Alexandru Voiculescu <bogdanx.a.voiculescu>
Severity: normal    
Priority: Medium CC: bogdanx.a.voiculescu, diego.ml, dvhart, jessica.zhang, sgw
Version: 1.5   
Target Milestone: 2.2 M2   
Hardware: Other   
OS: arm   
See Also: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7137
Whiteboard:
OS type for building Yocto: --- Type of Regression: ---
Verified: Documentation change: No (bug/feature does not impact docs)

Description Davide Soldan 2013-10-16 14:38:45 UTC
branch: dora
commit id (meta-yocto): 56f39bcf2237c11508d82238b6292e4bfbfed764
          (meta-fsl-arm): 00f0cf377224829f450d99bdc4664ffe96623996
          (meta-fsl-arm-extra): 12af63db7719877fd191ed2aa921a455a04f677b
          (meta-fsl-demos): 393b0119d513f9abf7b0570ab0d32b81583dedca

Hi,
I'm running an image that inherit core-image on a nitrogen6x.
Running su - USER -c 'env' lead to different environment variables than running su - USER, pressing enter and then running env from the new shell.

In /etc/profile: PATH="/usr/local/bin:/usr/bin:/bin:/opt/java/bin"
su - USER -c 'env': PATH=/bin:/usr/bin
su - USER, press enter, env: PATH=/usr/local/bin:/usr/bin:/bin:/opt/java/bin

so it seems that su - USER -c 'env' doesn't read /etc/profile, but it should do so...
Comment 1 Darren Hart 2013-10-17 15:05:02 UTC
If you specify -l (login) to su, is /etc/profile read? /etc/profile as far as I understand it is only read for login shells. BASH(1) confirms this.
Comment 2 Davide Soldan 2013-10-18 09:41:34 UTC
(In reply to comment #1)
> If you specify -l (login) to su, is /etc/profile read? /etc/profile as far
> as I understand it is only read for login shells. BASH(1) confirms this.

"su - USER" is the same as "su -l USER" and "make the shell a login shell". So "su - USER -c 'env'" is the same as "su -l USER -c 'env'". I've done a test that confirm this.../etc/profile is not read by none of the two commands...
Comment 3 Saul Wold 2013-10-22 14:32:18 UTC
Some more information is needed, is this happening with Busybox or Shadow 
version of su?  I re-tested this again with busybox (I might have found a 
different problem with shadow tools, which I will retest when I rebuild.
root@qemux86-64:~# grep PATH /etc/profile
PATH="/usr/local/bin:/usr/bin:/bin:/opt/java/bin"
   PATH=$PATH:/usr/local/sbin:/usr/sbin:/sbin
export PATH PS1 OPIEDIR QPEDIR QTDIR EDITOR TERM
root@qemux86-64:~# echo $PATH
/usr/local/bin:/usr/bin:/bin:/opt/java/bin:/usr/local/sbin:/usr/sbin:/sbin
root@qemux86-64:~# su - test -c env
USER=test
HOME=/home/test
PS1=\h:\w\$ 
LOGNAME=test
TERM=linux
PATH=/usr/local/bin:/usr/bin:/bin:/opt/java/bin
SHELL=/bin/sh
PWD=/home/test
TZ=UTC
EDITOR=/bin/vi
root@qemux86-64:~# su - test
qemux86-64:~$ env
USER=test
HOME=/home/test
PS1=\h:\w\$ 
LOGNAME=test
TERM=linux
PATH=/usr/local/bin:/usr/bin:/bin:/opt/java/bin
SHELL=/bin/sh
PWD=/home/test
TZ=UTC
EDITOR=/bin/vi
qemux86-64:~$ root@qemux86-64:~# ls -l /bin/su
lrwxrwxrwx    1 root     root            17 Oct 22 03:27 /bin/su -> 
/bin/busybox.suid
root@qemux86-64:~#
Comment 4 Davide Soldan 2013-10-22 15:21:48 UTC
(In reply to comment #3)
> Some more information is needed, is this happening with Busybox or Shadow 
> version of su?  I re-tested this again with busybox (I might have found a 
> different problem with shadow tools, which I will retest when I rebuild.

nitrogen6x:~$ ls -l /bin/su
lrwxrwxrwx    1 root     root            14 Oct 21 13:21 /bin/su -> /bin/su.shadow
root@nitrogen6x:~# rpm -qf /bin/su.shadow
shadow-4.1.4.3-r13.cortexa9hf_vfp_neon

so it's the shadow's one.

I've got busybox installed so I thought I could use su from busybox instead of su.shadow. But changing /bin/su from /bin/su.shadow to /bin/busybox.suid doesn't solve the problem. The behavior of su - USER -c 'env' is the same...
Actually with busybox --list I don't see su but only sulogin as defined function...
Comment 5 Saul Wold 2013-12-18 08:10:27 UTC
This hit at kind of a crazy time, I am getting back to this again, just an FYI 
regarding your looking for the su app in busybox, you need to look into 
busybox.suid instead.

Based on looking at what busybox does with su -c, it passes the -c option to 
/bin/sh which causes it to be non-interactive, thus not reading the 
/etc/profile file, this behaviour is consistent with both the busybox and 
shadow versions of su, I also tested it with busybox and bash and got the same 
results when using oe-core based tools.

I am not sure this is really a bug, but will not close until I hear back from 
you.
Comment 6 Saul Wold 2014-01-14 19:14:09 UTC
Since I have not heard back, I am closing this out, please reopen and provide 
updates if we still feel this is a bug.
Comment 7 Davide Soldan 2014-01-15 16:27:24 UTC
Hi Saul,
excuse me for the late reply.

I'm a little confused: from this page
https://www.gnu.org/software/bash/manual/html_node/Bash-Startup-Files.html
under "Invoked non-interactively" section, I can read:
"if a non-interactive shell is invoked with the --login option, Bash attempts to read and execute commands from the login shell startup files"
So in my case "su - USER -c COMMAND" leads to a non interactive shell, but the "- USER" option causes it to be a login shell. So this login and non-interactive shell should read informations from login shell startup files (es: /etc/profile)..
Do you agree with that?

- where did you find the information about "[...] what busybox does with su -c, it passes the -c option to /bin/sh which causes it to be non-interactive [...]" ? 

Thanks!
Comment 8 Saul Wold 2014-01-15 23:03:45 UTC
I did some testing with strace and looked at the execve call for the shell 
call, the out put from strace shows what gets passed to the shell.

In both cases shadow.su and busybox.su, the -c was passed:

execve("/bin/su", ["su", "-", "t", "-c", "env"], [/* 14 vars */]) = 0
execve("/bin/sh", ["-sh", "-c", "env"], [/* 6 vars */]) = 0

execve("/bin/su", ["su", "-l", "t", "-c", "env"], [/* 14 vars */]) = 0
execve("/bin/sh", ["-sh", "-c", "env"], [/* 6 vars */]) = 0

execve("/bin/su", ["su", "-", "t"], [/* 14 vars */]) = 0
execve("/bin/sh", ["-sh"], [/* 6 vars */]) = 0

execve("/bin/su", ["su", "-l", "t"], [/* 14 vars */]) = 0
execve("/bin/sh", ["-sh"], [/* 6 vars */]) = 0


Note the --login option does not get passed to bash from su in either case, I 
also verified this on my desktop running Fedora, but on my desktop it does read 
the /etc/profile, I am not sure if this is a fedora'ism I am looking at the 
bash and busybox shell code now
Comment 9 Chen Qi 2015-05-19 01:42:35 UTC
Module: openembedded-core.git
Branch: master
Commit: 6820f05dad0b4f9b9bbcf7c2a0af8c34f66199ae
URL:    http://git.openembedded.org/?p=openembedded-core.git&a=commit;h=6820f05dad0b4f9b9bbcf7c2a0af8c34f66199ae

Author: Chen Qi <Qi.Chen@windriver.com>
Date:   Tue Apr 21 17:30:46 2015 +0800

shadow: fix `su' behaviour

0001-su.c-fix-to-exec-command-correctly.patch is removed. Below is the reason.
This patch is introduced to solve the 'su: applet not found' problem when
executing `su -l xxx -c env'. The patch references codes of previous release
of shadow. However, this patch introduces bug#5359. So it's not correct.

Let's first look at the root cause of 'su: applet not found' problem.
This problem appears when /bin/sh is provided by busybox.
When executing `su -l xxx -c env' command, the following function is invoked.
    execve("/bin/sh", ["-su", "-c", "env"], [/* 6 vars */])
Note that the argv[0] provided to new executable file (/bin/sh) is "-su".
As /bin/sh is a symlink to /bin/busybox. It's /bin/busybox that is executed.
In busybox's appletlib.c, it would examine argv[0], try to find an applet
that has the same name, and then try to execute the main function of the
applet. This logic results in `su' applet from busybox to be executed.
However, we default to set 'BUSYBOX_SPLIT_SUID' to "1", so 'su' is not found.
Further more, even if we set 'BUSYBOX_SPLIT_SUID' to "0", so that 'su' applet
is found. The whole behaviour is still not correct. Because 'su' from shadow
takes higher priority than that from busybox, so 'su' from busybox should never
be executed on such system unless it's specified clearly by the end user.
The logic of busybox's appletlib.c is totally correct from the point of busybox
itself. It's an integration problem.

To solve the above problem, this patch comment out SU_NAME in /etc/login.defs
so that the final function executed in shadow's su is as below.
    execve("/bin/sh", ["-sh", "-c", "env"], [/* 6 vars */])

[YOCTO #5359]
[YOCTO #7137]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Comment 10 Bogdan Alexandru Voiculescu 2015-10-13 13:44:31 UTC
verified on master c1df471feacaf2590216aa476ce242908dac38cf;
After building a core-image-minimal + shadow package; problem does not reproduce
Comment 11 Diego 2016-04-20 13:59:12 UTC
The patch might fix the problem for busybox, but seems to create it for bash:

root@localhost:~# ls -l /bin/sh
lrwxrwxrwx 1 root root 9 Apr 11 15:32 /bin/sh -> /bin/bash
root@localhost:~# grep "SU_NAME" /etc/login.defs
#SU_NAME                su
root@localhost:~# su - -c 'env' | grep PATH
PATH=/sbin:/bin:/usr/sbin:/usr/bin
root@localhost:~# sed -i 's:^#SU_NAME:SU_NAME:g' /etc/login.defs
root@localhost:~# grep "SU_NAME" /etc/login.defs
SU_NAME         su
root@localhost:~# su - -c 'env' | grep PATH
PATH=/usr/local/bin:/usr/bin:/bin:/opt/java/bin:/usr/local/sbin:/usr/sbin:/sbin

Has anyone tested using bash?
Comment 12 Chen Qi 2016-04-27 07:32:01 UTC
(In reply to comment #11)
> The patch might fix the problem for busybox, but seems to create it for bash:
> 
> root@localhost:~# ls -l /bin/sh
> lrwxrwxrwx 1 root root 9 Apr 11 15:32 /bin/sh -> /bin/bash
> root@localhost:~# grep "SU_NAME" /etc/login.defs
> #SU_NAME                su
> root@localhost:~# su - -c 'env' | grep PATH
> PATH=/sbin:/bin:/usr/sbin:/usr/bin
> root@localhost:~# sed -i 's:^#SU_NAME:SU_NAME:g' /etc/login.defs
> root@localhost:~# grep "SU_NAME" /etc/login.defs
> SU_NAME         su
> root@localhost:~# su - -c 'env' | grep PATH
> PATH=/usr/local/bin:/usr/bin:/bin:/opt/java/bin:/usr/local/sbin:/usr/sbin:/
> sbin
> 
> Has anyone tested using bash?

I did some tests. Things are OK with bash according to my results.
I added 'PATH=$PATH:/home/test' to /etc/profile after system booted up for testing. And below are the results.

root@qemux86-64:~# vi /etc/profile 
root@qemux86-64:~# su - -c 'env'
SHELL=/bin/sh
TERM=xterm
USER=root
MAIL=/var/spool/mail/root
PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/test
PWD=/home/root
EDITOR=vi
TZ=UTC
SHLVL=1
HOME=/home/root
LOGNAME=root
_=/usr/bin/env
root@qemux86-64:~# ls -l /bin/sh
lrwxrwxrwx    1 root     root             9 Apr 27 07:00 /bin/sh -> /bin/bash
Comment 13 Diego 2016-04-27 12:34:17 UTC
(In reply to comment #12)
> 
> I did some tests. Things are OK with bash according to my results.
> I added 'PATH=$PATH:/home/test' to /etc/profile after system booted up for
> testing. And below are the results.
> 
> root@qemux86-64:~# vi /etc/profile 
> root@qemux86-64:~# su - -c 'env'
> SHELL=/bin/sh
> TERM=xterm
> USER=root
> MAIL=/var/spool/mail/root
> PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/home/test
> PWD=/home/root
> EDITOR=vi
> TZ=UTC
> SHLVL=1
> HOME=/home/root
> LOGNAME=root
> _=/usr/bin/env
> root@qemux86-64:~# ls -l /bin/sh
> lrwxrwxrwx    1 root     root             9 Apr 27 07:00 /bin/sh -> /bin/bash


I must specify that I'm still on fido, but it looks like the fix has been backported and is included in my build.
http://cgit.openembedded.org/openembedded-core/commit/meta/recipes-extended/shadow?h=fido&id=c7ba25a1e2fd36789ad6f55f05b41c3dc9b7f089

How do you suggest to debug the problem further?
Comment 14 Chen Qi 2016-04-28 01:48:46 UTC
Is the following patch in your branch?
I might forget to add it in comment at that time ...




If NON_INTERACTIVE_LOGIN_SHELLS is defined, all login shells read the
startup files, even if they are not interactive.

This is the behaviour of other major distros like Ubuntu and Fedora.
We also need to set it so that when executing `su -l xxx -c env' command,
/etc/profile is parsed.

[YOCTO #5359]
[YOCTO #7137]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-extended/bash/bash.inc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/meta/recipes-extended/bash/bash.inc b/meta/recipes-extended/bash/bash.inc
index d7b28cf..c06f157 100644
--- a/meta/recipes-extended/bash/bash.inc
+++ b/meta/recipes-extended/bash/bash.inc
@@ -9,6 +9,11 @@ inherit autotools gettext texinfo update-alternatives ptest
 EXTRA_AUTORECONF += "--exclude=autoheader"
 EXTRA_OECONF = "--enable-job-control"
 
+# If NON_INTERACTIVE_LOGIN_SHELLS is defined, all login shells read the
+# startup files, even if they are not interactive.
+# This is what other major distros do. And this is what we wanted. See bug#5359 and bug#7137.
+CFLAGS += "-DNON_INTERACTIVE_LOGIN_SHELLS"
+
 ALTERNATIVE_${PN} = "sh"
 ALTERNATIVE_LINK_NAME[sh] = "${base_bindir}/sh"
 ALTERNATIVE_TARGET[sh] = "${base_bindir}/bash"
--
Comment 15 Diego 2016-04-28 12:59:47 UTC
(In reply to comment #14)
> Is the following patch in your branch?
> I might forget to add it in comment at that time ...
> 
> 
> 
> 
> If NON_INTERACTIVE_LOGIN_SHELLS is defined, all login shells read the
> startup files, even if they are not interactive.
> 

It looks like you nailed the problem:
http://cgit.openembedded.org/openembedded-core/log/meta/recipes-extended/bash/bash.inc?h=jethro
http://cgit.openembedded.org/openembedded-core/log/meta/recipes-extended/bash/bash.inc?h=fido

The commit you mention is in jethro but not in fido. I've cherry-picked it and now PATH is correct even using 'su - -c env'.

Hope this is also officially backportable to fido.

Thank you very much for the patches and your support Chen Qi.
Comment 16 Chen Qi 2016-07-12 05:14:50 UTC
As mentioned in the above commit, problem has been solved.
Close this bug.