Skip to content

Commit 6d63f2a

Browse files
Kernel+LibC: Remove ENOTSUP fallback for file actions
All posix_spawn file actions are now implemented in the kernel: - dup2, close, open, chdir, fchdir The ENOTSUP fallback path in LibC is no longer needed. Errors from file actions are now returned directly from the syscall. The fork() path is still used when spawnattr is present (FIXME).
1 parent 7fb91e0 commit 6d63f2a

File tree

3 files changed

+4
-30
lines changed

3 files changed

+4
-30
lines changed

Kernel/API/Syscall.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,6 @@ struct SC_posix_spawn_params {
495495
size_t attr_data_size;
496496
Userspace<void const*> serialized_file_actions_data;
497497
size_t serialized_file_actions_data_size;
498-
u8 file_action_types_present;
499498
};
500499

501500
struct SC_futimens_params {

Kernel/Syscalls/posix_spawn.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ ErrorOr<void> Process::execute_file_actions(ReadonlyBytes file_actions_data)
4747

4848
auto description = TRY(fds.open_file_description(action->old_fd));
4949
if (action->old_fd != action->new_fd) {
50+
5051
if (fds.m_fds_metadatas[action->new_fd].is_allocated()) {
5152
if (auto* old_description = fds[action->new_fd].description())
5253
(void)old_description->close();
@@ -109,6 +110,7 @@ ErrorOr<void> Process::execute_file_actions(ReadonlyBytes file_actions_data)
109110
}));
110111
break;
111112
}
113+
112114
case SpawnFileActionType::Chdir: {
113115
if (header->record_length < sizeof(SpawnFileActionChdir))
114116
return EINVAL;
@@ -134,7 +136,7 @@ ErrorOr<void> Process::execute_file_actions(ReadonlyBytes file_actions_data)
134136
if (!description->is_directory())
135137
return ENOTDIR;
136138

137-
// Check for search (+x) permission on the directory
139+
// Check for search (+x) permission on the directory.
138140
if (!description->metadata().may_execute(credentials()))
139141
return EACCES;
140142

@@ -174,10 +176,6 @@ ErrorOr<FlatPtr> Process::sys$posix_spawn(Userspace<Syscall::SC_posix_spawn_para
174176
// Copy file actions buffer from userspace
175177
OwnPtr<KBuffer> file_actions_buffer;
176178
if (params.serialized_file_actions_data.ptr() != 0 && params.serialized_file_actions_data_size != 0) {
177-
// Check for unsupported file actions
178-
constexpr u8 SUPPORTED_SPAWN_ACTIONS = (1 << static_cast<u8>(SpawnFileActionType::Dup2)) | (1 << static_cast<u8>(SpawnFileActionType::Close)) | (1 << static_cast<u8>(SpawnFileActionType::Open)) | (1 << static_cast<u8>(SpawnFileActionType::Chdir)) | (1 << static_cast<u8>(SpawnFileActionType::Fchdir));
179-
if (params.file_action_types_present & ~SUPPORTED_SPAWN_ACTIONS)
180-
return ENOTSUP;
181179

182180
if (params.serialized_file_actions_data_size > 1 * MiB)
183181
return E2BIG;

Userland/Libraries/LibC/spawn.cpp

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
struct posix_spawn_file_actions_state {
3030
ByteBuffer buffer;
31-
u8 action_types_present { 0 };
3231
};
3332

3433
extern "C" {
@@ -196,11 +195,9 @@ static ErrorOr<pid_t> posix_spawn_syscall(char const* path, posix_spawn_file_act
196195
if (file_actions && !file_actions->state->buffer.is_empty()) {
197196
posix_spawn_params.serialized_file_actions_data = file_actions->state->buffer.data();
198197
posix_spawn_params.serialized_file_actions_data_size = file_actions->state->buffer.size();
199-
posix_spawn_params.file_action_types_present = file_actions->state->action_types_present;
200198
} else {
201199
posix_spawn_params.serialized_file_actions_data = nullptr;
202200
posix_spawn_params.serialized_file_actions_data_size = 0;
203-
posix_spawn_params.file_action_types_present = 0;
204201
}
205202

206203
pid_t rc = syscall(SC_posix_spawn, &posix_spawn_params);
@@ -229,23 +226,8 @@ int posix_spawn(pid_t* out_pid, char const* path, posix_spawn_file_actions_t con
229226
}
230227

231228
auto child_pid_or_error = posix_spawn_syscall(path, file_actions, argv, envp);
232-
if (child_pid_or_error.is_error()) {
233-
// ENOTSUP means kernel doesn't support these file actions yet, fall back to fork()
234-
if (child_pid_or_error.error().code() == ENOTSUP) {
235-
pid_t child_pid = fork();
236-
if (child_pid < 0)
237-
return errno;
238-
239-
if (child_pid != 0) {
240-
if (out_pid)
241-
*out_pid = child_pid;
242-
return 0;
243-
}
244-
245-
posix_spawn_child(path, file_actions, attr, argv, envp, execve);
246-
}
229+
if (child_pid_or_error.is_error())
247230
return child_pid_or_error.error().code();
248-
}
249231

250232
if (out_pid)
251233
*out_pid = child_pid_or_error.value();
@@ -315,7 +297,6 @@ int posix_spawn_file_actions_addchdir(posix_spawn_file_actions_t* actions, char
315297

316298
if (actions->state->buffer.try_append(record_buffer.data(), record_size).is_error())
317299
return ENOMEM;
318-
actions->state->action_types_present |= (1 << static_cast<u8>(SpawnFileActionType::Chdir));
319300
return 0;
320301
}
321302

@@ -328,7 +309,6 @@ int posix_spawn_file_actions_addfchdir(posix_spawn_file_actions_t* actions, int
328309
action.fd = fd;
329310
if (actions->state->buffer.try_append(&action, sizeof(action)).is_error())
330311
return ENOMEM;
331-
actions->state->action_types_present |= (1 << static_cast<u8>(SpawnFileActionType::Fchdir));
332312
return 0;
333313
}
334314

@@ -342,7 +322,6 @@ int posix_spawn_file_actions_addclose(posix_spawn_file_actions_t* actions, int f
342322
action.fd = fd;
343323
if (actions->state->buffer.try_append(&action, sizeof(action)).is_error())
344324
return ENOMEM;
345-
actions->state->action_types_present |= (1 << static_cast<u8>(SpawnFileActionType::Close));
346325
return 0;
347326
}
348327

@@ -357,7 +336,6 @@ int posix_spawn_file_actions_adddup2(posix_spawn_file_actions_t* actions, int ol
357336
action.new_fd = new_fd;
358337
if (actions->state->buffer.try_append(&action, sizeof(action)).is_error())
359338
return ENOMEM;
360-
actions->state->action_types_present |= (1 << static_cast<u8>(SpawnFileActionType::Dup2));
361339
return 0;
362340
}
363341

@@ -386,7 +364,6 @@ int posix_spawn_file_actions_addopen(posix_spawn_file_actions_t* actions, int wa
386364

387365
if (actions->state->buffer.try_append(record_buffer.data(), record_size).is_error())
388366
return ENOMEM;
389-
actions->state->action_types_present |= (1 << static_cast<u8>(SpawnFileActionType::Open));
390367
return 0;
391368
}
392369

0 commit comments

Comments
 (0)