illumos gate

In the original code, there are a number of problems contributing to this bad behaviour.

First, the path_xattr() function is supposed to update any buffer passed in as its third argument with the resolved path as a side effect of checking if a path is listed in exec_attr without any attributes. It fails because it resolves the path using a local variable and does not touch the passed-in path.

Second, if path resolution fails (or is never run) - i.e. path_xattr() returns -1 - then path_pfexecve() ploughs on anyway, trying to use pfexec to launch whatever junk is in the resolvedpath stack variable. This happens for any command which is not found via the exec_attr lookup.

Thirdly, the path_pfexecve() function uses vfork() and it should not. The only safe thing to do after vfork() is an exec, and this code does a lot more than that.

Following discussion on IRC, rather than trying to fix the shell's built-in profile shell handling, I am going to completely disable it. Paths such as /usr/bin/pfksh are hardlinks to /usr/bin/pfexec so are already using the New implementation of pfexec(1) and all of the profile shells.

Having rebuilt ksh without the SHOPT_PFOPT flag, I've tested pfsh in the global and non-global zone and everything works as expected.

For example:

bloody# su - bob
The illumos Project     gate-ig_13557_pfksh-fe34df72a4d February 2021
bob@bloody:~$
bob@bloody:~$
bob@bloody:~$ profiles
File System Management
SMB Management
VSCAN Management
SMBFS Management
Basic Solaris User
All
bob@bloody:~$
bob@bloody:~$ /usr/bin/mkdir /root/fff
mkdir: Failed to make directory "/root/fff"; Permission denied
bob@bloody:~$
bob@bloody:~$ pfsh
bob@bloody:~$ /usr/bin/mkdir /root/fff
bob@bloody:~$ /usr/bin/touch /root/fff
touch: cannot stat /root/fff: Permission denied
bob@bloody:~$

As with the previous ksh version, builtins still get in the way (which was one of the improvements in the newer ksh, but given the state of the pfsh code, I don't want to pursue that option)

bob@build:~$ pfsh
bob@build:~$ mkdir /root/fff
mkdir: /root/fff: [Permission denied]
bob@build:~$ /usr/bin/mkdir /root/fff
bob@build:~$

Sorry I didn't get a chance to comment on this earlier, but this patch still doesn't work for me, but the behavior is different from the original.

[root@87740fdf-65ff-6aeb-a5d0-baa4db5b7ecb ~]# cat test.sh 
#!/usr/bin/pfksh

/bin/echo This will fail
ls -1 / | /usr/bin/head

/bin/echo  This will succeed
/bin/ls -1 / | /usr/bin/head
[root@87740fdf-65ff-6aeb-a5d0-baa4db5b7ecb ~]# ./test.sh 
This will fail
pfexec: unable to execute ???????4??????ҁ#????`
: No such file or directory
This will succeed
bin
checkpoints
dev
etc
home
lib
opt
proc
root
sbin

Is that with the final patch that got committed?

I could replicate your original problem in a non-global zone, but the latest patch works for me in the GZ and NGZ (and disables the shell's built-in profile shell handling entirely).

I have managed to replicate this now and will get this fixed in gate. The patch that was committed to gate was missing a couple of small pieces.

git commit 6859ab0054a96093394fe85f055fac9cf93b38ee

commit  6859ab0054a96093394fe85f055fac9cf93b38ee
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2021-02-21T21:57:06.000Z

    13557 pf[k]sh fails to properly resolve paths (missing diffs)
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Reviewed by: Dominik Hassler <hadfl@omnios.org>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Confirmed, with the second patch it now works.

Thanks!