Anonymous | Login | 2024-12-02 08:05 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||
ID | Category | Severity | Type | Date Submitted | Last Update | ||
0001208 | [1003.1(2016/18)/Issue7+TC2] System Interfaces | Objection | Enhancement Request | 2018-09-07 18:39 | 2024-06-11 09:08 | ||
Reporter | eblake | View Status | public | ||||
Assigned To | |||||||
Priority | normal | Resolution | Accepted As Marked | ||||
Status | Closed | ||||||
Name | Eric Blake | ||||||
Organization | Red Hat | ||||||
User Reference | posix_spawn.chdir | ||||||
Section | posix_spawn | ||||||
Page Number | 1452 ff. | ||||||
Line Number | 48227 ff. | ||||||
Interp Status | --- | ||||||
Final Accepted Text | Note: 0004832 | ||||||
Summary | 0001208: calling chdir as part of posix_spawn | ||||||
Description |
One notable missing feature from posix_spawn is the ability to change the initial working directory of the new process. Of course, in the common case, it is possible to exec a shim process that will change the directory (GNU coreutils has 'env -C dir program ...' that will do this, but the standard itself does not have such a shim program, so users have to come up with their own). But even more complex is the fact that posix_spawn can change access permissions with POSIX_SPAWN_RESETIDS, such that the parent may have permission to change to a particular directory, but the child with a different id cannot do so. There is also the case that a multi-threaded parent process can use openat() to open files relative to a particular directory without changing the global state of the current working directory, but there is no interface for openat() within posix_spawn (and it may be prohibitively expensive to convert a relative name into an absolute one). Thus, it can be argued that changing the directory needs to be directly part of posix_spawn(). Solaris has implemented an extension for chdir with a _np suffix (non-portable), and documents that relative names for posix_spawn(,path,,,,) and posix_spawnp(,file,,,,) are interpreted to the working directory selected for the child: https://docs.oracle.com/cd/E86824_01/html/E54766/posix-spawn-file-actions-addchdir-np-3c.html [^] I'm proposing that we standardize this, but drop the _np suffix (since by standardizing it, it will no longer be non-portable), and also add a counterpart for fchdir. The addition of fchdir makes it possible to simulate openat semantics without actually needing to add posix_spawn_file_actions_addopenat. Note that I did not standardize the notion of a chroot file action (in part due to no existing practice), although such an extension may still make sense. Fix a couple of issues in posix_spawn_file_addclose while at it: you can't delete open/close actions from an existing file_actions object, only add more; and ENOMEM should be a 'shall fail' rather than 'may fail' error to match posix_spawn_file_adddup2. Yes, the additions to the XRAT example are just as borked as the existing implementation for handling an addopen action at lines 126723-126763 and 126852-126860 (very easy to write beyond end of fixed-length array, parse failure on filename containing '*', failure to fail with EBADF for negative filedes, etc) - but being consistently bad in something that is mainly for demo purposes isn't the end of the world. The use of _exit() instead of exit() in the example matches the resolution for 0001044. |
||||||
Desired Action |
Proposed changes (2017 edition page and line numbers): On page 341 line 11588 (XBD <spawn.h>), insert int posix_spawn_file_actions_addchdir(posix_spawn_file_actions_t *restrict, const char *restrict); On page 341 line 11592 (<spawn.h>), insert int posix_spawn_file_actions_addfchdir(posix_spawn_file_actions_t *, int); On page 1453 line 48227 (XSH posix_spawn DESCRIPTION), change: a pathname that identifies the new process image file to execute.to: a pathname that identifies the new process image file to execute; if the pathname does not start with a <slash> it shall be interpreted relative to the working directory of the child process after all file_actions have been performed. On page 1453 line 48229, change: the file parameter shall be used as the pathname for the new process image file.to: the file parameter shall be used as the pathname for the new process image file; if this pathname does not start with a <slash> it shall be interpreted relative to the working directory of the child process after all file_actions have been performed. On page 1453 line 48239, add a sentence to the paragraph: The current working directory of the child process shall be the same as it is in the parent process. On page 1453 line 48250, change: The file actions specified by the spawn file actions object shall be performed in the order in which they were added to the spawn file actions object.to: The file actions specified by the spawn file actions object shall be performed in the order in which they were added to the spawn file actions object, and relative file names in a given action shall be interpreted in relation to the tracked working directory. The tracked working directory shall begin with the current working directory of the parent process, and may be altered according to chdir or fchdir file actions; the current working directory of the child process shall be the final state of the tracked working directory after all file actions have been applied. On page 1455 line 48346 (posix_spawn ERRORS), change: If the file_actions argument is not NULL, and specifies any close, dup2, or open actions to be performed, and if posix_spawn( ) or posix_spawnp( ) fails for any of the reasons that would cause close( ), dup2( ), or open( ) to fail, an error value shall be returned as described by close( ), dup2( ), and open( ), respectivelyto: If the file_actions argument is not NULL, and specifies any chdir, close, dup2, fchdir, or open actions to be performed, and if posix_spawn( ) or posix_spawnp( ) fails for any of the reasons that would cause chdir( ), close( ), dup2( ), fchdir( ), or open( ) to fail, an error value shall be returned as described by chdir( ), close( ), dup2( ), fchdir( ), and open( ), respectively On page 1456 line 48380 (posix_spawn RATIONALE), change: control of six types of inheritance: file descriptors,to: control of seven types of inheritance: file descriptors, current working directory, On page 1456 line 48386, insert a sentence between the two existing sentences: Control of the current working directory is required because the parent process may want to constrain the resources that the child process can reach from its current working directory or affect how relative pathnames are interpreted, while recognizing that a multi-threaded parent process would require a lot of overhead to safely change its own working directory prior to creating the child process. On page 1459 line 48518 (posix_spawn SEE ALSO), add in sorted order: posix_spawn_file_actions_addchdir( ), On page 1460, insert a new interface: posix_spawn_file_actions_addchdir( ) On page 1460 line 48545 (posix_spawn_file_actions_addclose DESCRIPTION), remove: or delete On page 1460 line 48565, add a sentence: A relative path shall be interpreted in relation to the working directory determined by any prior actions. On page 1461 line 48576 (posix_spawn_file_actions_addclose ERRORS), insert: These functions shall fail if: On page 1461 line 48578, delete: [ENOMEM] Insufficient memory exists to add to the spawn file actions object. On page 1461 line 48597 (posix_spawn_file_actions_addclose RATIONALE), change: A spawn file actions object may be initialized to contain an ordered sequence of close( ), dup2( ), and open( ) operations to be used by posix_spawn( ) or posix_spawnp( ) to arrive at the set of open file descriptors inherited by the spawned process from the set of open file descriptors in the parent at the time of the posix_spawn( ) or posix_spawnp( ) call.to: A spawn file actions object may be initialized to contain an ordered sequence of chdir( ), close( ), dup2( ), fchdir( ), and open( ) operations to be used by posix_spawn( ) or posix_spawnp( ) to arrive at the set of open file descriptors and current working directory inherited by the spawned process from the set of open file descriptors and current working directory in the parent at the time of the posix_spawn( ) or posix_spawnp( ) call. On page 1462 line 48654 (posix_spawn_file_actions_addclose SEE ALSO), add in sorted order: posix_spawn_file_actions_addchdir( ), On page 1465, insert a new forwarding page: posix_spawn_file_actions_addfchdir( ) On page 3695 line 126592 (XRAT B.3.3 Examples for Spawn), insert: int posix_spawn_file_actions_addchdir( posix_spawn_file_actions_t *restrict file_actions, const char *restrict path); On page 3695 line 126597, insert: int posix_spawn_file_actions_addfchdir( posix_spawn_file_actions_t *file_actions, int filedes); On page 3699 line 126764, insert with correct indentation: else if (strncmp(p, "chdir(", 6) == 0) { char path[1000]; /* Should be dynamic */ char *q; p += 6 q = strchr(p, '*'); if (q == NULL) _exit(127); strncpy(path, p, q - p); path[q - p] = '\0'; if (chdir(path) == -1) _exit(127); } else if (strncmp(p, "fchdir(", 7) == 0) { int fd; if (sscanf(p + 7, "%d)", &fd) != 1) _exit(127); if (fchdir(fd) == -1) _exit(127); } On page 3700 line 126835, insert with correct indentation: /* Add a chdir action to object. */ int posix_spawn_file_actions_addchdir( posix_spawn_file_actions_t *restrict file_actions, const char *restrict path) { char temp[100]; sprintf(temp, "chdir(%s*)", path); return add_to_file_actions(file_actions, temp); } /* Add a fchdir action to object. */ int posix_spawn_file_actions_addfchdir( posix_spawn_file_actions_t *file_actions, int filedes) { char temp[100]; sprintf(temp, "fchdir(%d)", filedes); return add_to_file_actions(file_actions, temp); } |
||||||
Tags | issue8 | ||||||
Attached Files | |||||||
|
Relationships | |||||||||||||||||||
|
Notes | |
(0004109) eblake (manager) 2018-09-07 19:02 |
Perhaps the two cleanups in posix_spawn_file_actions_addclose (deleting a spurious "or delete", and turning ENOMEM into shall fail instead of may fail) should be split into a separate bug targeted at TC3. Also, such a separate bug could add even more disclaimers into the XRAT example code about how insecure it is, and that it exists merely as a starting point rather than a robust implementation. |
(0004110) eblake (manager) 2018-09-07 22:18 |
As a side note: Windows CreateProcess() already supports changing directories at the time of spawning a child. While you wouldn't typically see posix_spawn() and CreateProcess() mixed in the same program, it is nice to know that the concept of changing directory for the child process is already common in an existing OS. |
(0004111) eblake (manager) 2018-09-07 22:44 edited on: 2018-09-07 22:58 |
In fact, we could get by with JUST posix_spawn_file_actions_addfchdir(posix_spawn_file_actions_t*, int), since: posix_spawn_file_actions_addchdir(&act, "foo"); can be rewritten as: posix_spawn_file_actions_addopen(&act, 5, "foo", O_RDONLY|O_DIRECTORY|O_CLOEXEC, 0); posix_spawn_file_actions_addfchdir(&act, 5); modulo any issues with running out of available fds. Since the existing Solaris implementation uses const char* for the _addchdir() name, we'd definitely want to spell things as addfchdir() for an int version rather than risk confusion why addchdir() takes a different type than addchdir_np(). But with that said, here's an exploratory patch originally proposed to glibc implementing an fchdir action under the name addchdir() (but which never went anywhere at the time) - 8 years ago! https://sourceware.org/ml/libc-alpha/2010-08/msg00107.html [^] |
(0004353) eblake (manager) 2019-04-04 15:31 |
https://sourceware.org/bugzilla/show_bug.cgi?id=17405 [^] documents that glibc introduced posix_spawn_file_actions_addfchdir_np() in Dec 2018. (The _np suffix was intentional meaning "non-portable", in case any semantics have to change slightly to match what we decide here; a simple alias rename is the obvious action if everything matches) |
(0004354) eblake (manager) 2019-04-04 15:46 |
Based on discussion in the 2019-04-04 call, and with original poster's consent, the desired action was updated in-place with an edit to this paragraph:File actions are performed in a new process created by posix_spawn( ) or posix_spawnp( ) in the same order that they were added to the file actions object. Thus, the execution of an addopen action that was created by a call to posix_spawn_file_actions_addopen( ) that specifies a relative path will be affected by the execution of a chdir or fchdir action that was created by a previous call to posix_spawn_file_actions_addchdir( ) or posix_spawn_file_actions_addfchdir( ). Likewise, a relative path passed to posix_spawn( ) will be affected by the last chdir or fchdir action in the file action list. The paragraph still occurs in the text proposed to be added in the new interface addition after page 1460, but was moved from DESCRIPTION to APPLICATION USAGE. |
(0004830) geoffclare (manager) 2020-04-27 09:08 edited on: 2020-04-27 15:27 |
Reopening because the following change to posix_spawnp() is incorrect. On page 1453 line 48229, change: the file parameter shall be used as the pathname for the new process image file.to:
It should be something like the following. On page 1452 line 48232, change: a search of the directories passed as the environment variable PATH (see XBD Chapter 8, on page NNN).to: a search of the directories passed as the environment variable PATH (see XBD Chapter 8, on page NNN), using the working directory of the child process after all file_actions have been performed. |
(0004832) geoffclare (manager) 2020-04-27 15:33 |
Implement the desired action, as amended by Note: 0004830 |
Issue History | |||
Date Modified | Username | Field | Change |
2018-09-07 18:39 | eblake | New Issue | |
2018-09-07 18:39 | eblake | Name | => Eric Blake |
2018-09-07 18:39 | eblake | Organization | => Red Hat |
2018-09-07 18:39 | eblake | User Reference | => posix_spawn.chdir |
2018-09-07 18:39 | eblake | Section | => posix_spawn |
2018-09-07 18:39 | eblake | Page Number | => 1452 ff. |
2018-09-07 18:39 | eblake | Line Number | => 48227 ff. |
2018-09-07 18:39 | eblake | Interp Status | => --- |
2018-09-07 18:39 | eblake | Relationship added | related to 0001044 |
2018-09-07 18:44 | eblake | Desired Action Updated | |
2018-09-07 18:46 | eblake | Desired Action Updated | |
2018-09-07 18:49 | eblake | Desired Action Updated | |
2018-09-07 18:51 | eblake | Desired Action Updated | |
2018-09-07 18:53 | eblake | Desired Action Updated | |
2018-09-07 18:55 | eblake | Desired Action Updated | |
2018-09-07 18:55 | eblake | Tag Attached: issue8 | |
2018-09-07 19:02 | eblake | Note Added: 0004109 | |
2018-09-07 19:04 | eblake | Desired Action Updated | |
2018-09-07 19:29 | eblake | Desired Action Updated | |
2018-09-07 19:35 | eblake | Description Updated | |
2018-09-07 22:18 | eblake | Note Added: 0004110 | |
2018-09-07 22:44 | eblake | Note Added: 0004111 | |
2018-09-07 22:58 | eblake | Note Edited: 0004111 | |
2018-09-10 16:32 | eblake | Relationship added | related to 0000411 |
2019-04-04 15:31 | eblake | Note Added: 0004353 | |
2019-04-04 15:46 | eblake | Note Added: 0004354 | |
2019-04-04 15:47 | eblake | Desired Action Updated | |
2019-04-04 15:50 | nick | Status | New => Resolved |
2019-04-04 15:50 | nick | Resolution | Open => Accepted |
2020-04-27 09:08 | geoffclare | Note Added: 0004830 | |
2020-04-27 09:08 | geoffclare | Status | Resolved => Under Review |
2020-04-27 09:08 | geoffclare | Resolution | Accepted => Reopened |
2020-04-27 09:10 | geoffclare | Tag Detached: issue8 | |
2020-04-27 15:27 | geoffclare | Note Edited: 0004830 | |
2020-04-27 15:33 | geoffclare | Note Added: 0004832 | |
2020-04-27 15:34 | geoffclare | Final Accepted Text | => Note: 0004832 |
2020-04-27 15:34 | geoffclare | Status | Under Review => Resolved |
2020-04-27 15:34 | geoffclare | Resolution | Reopened => Accepted As Marked |
2020-04-27 15:35 | geoffclare | Tag Attached: issue8 | |
2020-05-19 11:01 | geoffclare | Status | Resolved => Applied |
2023-04-19 17:05 | eblake | Relationship added | related to 0001674 |
2024-06-11 09:08 | agadmin | Status | Applied => Closed |
Mantis 1.1.6[^] Copyright © 2000 - 2008 Mantis Group |