Review - 5798 fexecve() needed per POSIX 2008

Review - 5798 fexecve() needed per POSIX 2008

[Andy Fiddaman]

Hello,

I have a project coming up that would benefit from fexecve(2), and this
function is also part of POSIX 2008 so it .

I've taken Garrett's work in
    http://cr.illumos.org/~webrev/gdamore/fexecve/
and put it up for review. It takes an  approach of using a consed up
/dev/fd path versus introducing a new system call such as execveat()
in order to reduce risk and knock on impact to auditing etc. This seems
like a reasonable trade off to me but I'm interested in views from others.

I've made some changes to what was in that webrev, over and above what was
required to apply it to today's gate. Specifically:

 - Fixed a couple of bugs in the kernel code and expanded the comment;
 - Converted exec(2) to mdoc and re-applied Garrett's changes on top;
 - Fixed the waitpid(3C) call in the test program;
 - Addressed pbchk noise, apart from that in usr/src/lib/libxcurses/h/mks.h
   which I have added to the cstyle exception list.

Please can you review the following change?

    5798 fexecve() needed per POSIX 2008
    Author: Garrett D'Amore <garrett@damore.org>
    Portions contributed by: Andy Fiddaman <illumos@fiddaman.net>
    https://www.illumos.org/issues/5798
    https://code.illumos.org/c/illumos-gate/+/3229

Thanks,

Andy

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Cedric Blancher]

On Fri, 12 Jan 2024 at 15:06, Andy Fiddaman <illumos@fiddaman.net> wrote:
>
> Hello,
>
> I have a project coming up that would benefit from fexecve(2), and this
> function is also part of POSIX 2008 so it .
>
> I've taken Garrett's work in
>     http://cr.illumos.org/~webrev/gdamore/fexecve/
> and put it up for review. It takes an  approach of using a consed up
> /dev/fd path versus introducing a new system call such as execveat()
> in order to reduce risk and knock on impact to auditing etc. This seems
> like a reasonable trade off to me but I'm interested in views from others.

Unfortunately this is not going to work. The whole point of fexecve()
is to make tools like shells more efficient, but relying on /dev/fd
means that your shell will no longer work at boot time, or in a
chroot, when /dev/fd is not available/mounted yet.

Solaris 11 implements fexecve() has syscall, see
https://docs.oracle.com/cd/E26502_01/html/E29032/fexecve-2.html#scrolltoc

Ced
-- 
Cedric Blancher <cedric.blancher@gmail.com>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Robert Mustacchi]

On 1/12/24 16:45, Cedric Blancher wrote:
> On Fri, 12 Jan 2024 at 15:06, Andy Fiddaman <illumos@fiddaman.net> wrote:
>>
>> Hello,
>>
>> I have a project coming up that would benefit from fexecve(2), and this
>> function is also part of POSIX 2008 so it .
>>
>> I've taken Garrett's work in
>>     http://cr.illumos.org/~webrev/gdamore/fexecve/
>> and put it up for review. It takes an  approach of using a consed up
>> /dev/fd path versus introducing a new system call such as execveat()
>> in order to reduce risk and knock on impact to auditing etc. This seems
>> like a reasonable trade off to me but I'm interested in views from others.
> 
> Unfortunately this is not going to work. The whole point of fexecve()
> is to make tools like shells more efficient, but relying on /dev/fd
> means that your shell will no longer work at boot time, or in a
> chroot, when /dev/fd is not available/mounted yet.

There is a bit more nuance here in the implementation. It does not rely
on /dev/fd being mounted or present in a chroot (though that is its own
question given what's happening in the kernel implementation). So all
the cases you want to work should if you actually look at the
implementation.

Robert

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Bill Sommerfeld]

On 1/12/24 16:45, Cedric Blancher wrote:
> Unfortunately this is not going to work. The whole point of fexecve()
> is to make tools like shells more efficient, but relying on /dev/fd
> means that your shell will no longer work at boot time, or in a
> chroot, when /dev/fd is not available/mounted yet.

The proposed implementation does not rely on /dev/fd being mounted; 
instead, when it sees a "path" parameter beginning with the literal 
string "/dev/fd/" followed by a positive decimal integer and handles it 
as a special case.

						- Bill

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Cedric Blancher]

On Sat, 13 Jan 2024 at 01:57, Bill Sommerfeld via illumos-developer
<developer@lists.illumos.org> wrote:
>
> On 1/12/24 16:45, Cedric Blancher wrote:
> > Unfortunately this is not going to work. The whole point of fexecve()
> > is to make tools like shells more efficient, but relying on /dev/fd
> > means that your shell will no longer work at boot time, or in a
> > chroot, when /dev/fd is not available/mounted yet.
>
> The proposed implementation does not rely on /dev/fd being mounted;
> instead, when it sees a "path" parameter beginning with the literal
> string "/dev/fd/" followed by a positive decimal integer and handles it
> as a special case.

OK

What happens if you pass /proc/$randompid/fd/$fdnum ($shell variables
appropriately expanded :) )? That is legal on Linux, and while POSIX
does not define /proc, it is a legal case too

Ced
-- 
Cedric Blancher <cedric.blancher@gmail.com>
[https://plus.google.com/u/0/+CedricBlancher/]
Institute Pasteur

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Bill Sommerfeld]

On 1/12/24 17:52, Cedric Blancher wrote:
> On Sat, 13 Jan 2024 at 01:57, Bill Sommerfeld via illumos-developer
> <developer@lists.illumos.org> wrote:
>>
>> On 1/12/24 16:45, Cedric Blancher wrote:
>>> Unfortunately this is not going to work. The whole point of fexecve()
>>> is to make tools like shells more efficient, but relying on /dev/fd
>>> means that your shell will no longer work at boot time, or in a
>>> chroot, when /dev/fd is not available/mounted yet.
>>
>> The proposed implementation does not rely on /dev/fd being mounted;
>> instead, when it sees a "path" parameter beginning with the literal
>> string "/dev/fd/" followed by a positive decimal integer and handles it
>> as a special case.
> 
> OK
> 
> What happens if you pass /proc/$randompid/fd/$fdnum ($shell variables
> appropriately expanded :) )? That is legal on Linux, and while POSIX
> does not define /proc, it is a legal case too

What does that have to do with fexecve()?   You can't pass 
/proc/$randompid/fd/$fdnum to fexecve() -- it takes an integer file 
descriptor, not a pathname string.

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Andy Fiddaman]

On Sat, 13 Jan 2024, Cedric Blancher wrote:
> Solaris 11 implements fexecve() has syscall, see
> https://docs.oracle.com/cd/E26502_01/html/E29032/fexecve-2.html#scrolltoc

Solaris just has the 'execve' syscall, so they presumably extended it or did
something similar to this change. Of course Solaris and illumos diverged over a
decade ago so what they do doesn't have much impact on illumos.

The path to take here is a bit of a trade off and making "/dev/fd/" a special
string that the kernel spots does leave me a little uneasy. I am looking at
other options for implementing this but either way, as others have mentioned,
and is commented in the code itself, there is no reliance on "/dev/fd" or
"/proc" actually existing in the filesystem.

Andy

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Alan Coopersmith]

On 1/13/24 04:22, Andy Fiddaman wrote:
> On Sat, 13 Jan 2024, Cedric Blancher wrote:
>> Solaris 11 implements fexecve() has syscall, see
>> https://docs.oracle.com/cd/E26502_01/html/E29032/fexecve-2.html#scrolltoc
> 
> Solaris just has the 'execve' syscall, so they presumably extended it or did
> something similar to this change. 

Technically, Solaris 11 just has the execvex() syscall, which replaced
SYS_execve:

https://docs.oracle.com/cd/E88353_01/html/E37841/execvex-2.html

Despite having man pages in section 2, the other exec calls are just libc
wrappers around this actual system call.

	-alan-

Re: [developer] Review - 5798 fexecve() needed per POSIX 2008

[Jason King]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Also, keep in mind that this bit is a private interface between libc and the kernel. It can be changed and improved in the future without impacting any applications using fexecve().


From: Andy Fiddaman <illumos@fiddaman.net>
Date: Saturday, January 13, 2024 at 6:23 AM
To: illumos-developer <developer@lists.illumos.org>
Subject: Re: [developer] Review - 5798 fexecve() needed per POSIX 2008
On Sat, 13 Jan 2024, Cedric Blancher wrote:
> Solaris 11 implements fexecve() has syscall, see
> https://docs.oracle.com/cd/E26502_01/html/E29032/fexecve-2.html#scrolltoc

Solaris just has the 'execve' syscall, so they presumably extended it or did
something similar to this change. Of course Solaris and illumos diverged over a
decade ago so what they do doesn't have much impact on illumos.

The path to take here is a bit of a trade off and making "/dev/fd/" a special
string that the kernel spots does leave me a little uneasy. I am looking at
other options for implementing this but either way, as others have mentioned,
and is commented in the code itself, there is no reliance on "/dev/fd" or
"/proc" actually existing in the filesystem.

Andy


------------------------------------------
illumos: illumos-developer
Permalink: https://illumos.topicbox.com/groups/developer/T182498556842081b-M7abc3488313e0d1c65ad9f08
Delivery options: https://illumos.topicbox.com/groups/developer/subscription

[-- Attachment #2: Type: text/html, Size: 3726 bytes --]