Skip to content

Commit 100938b

Browse files
committed
Fix file descriptor leak when proc_open() descriptor setup fails
When a descriptor spec entry fails to set up (unknown type, missing mode) after an earlier entry already opened a pipe or socket, proc_open() jumped to exit_fail without closing the descriptors it had already opened, leaking those fds; repeated calls exhaust the process descriptor table. Close the opened descriptors at exit_fail and drop the now-redundant per-call close before each spawn-failure goto. Closes phpGH-22311
1 parent 98563c2 commit 100938b

2 files changed

Lines changed: 23 additions & 4 deletions

File tree

ext/standard/proc_open.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,7 +1371,6 @@ PHP_FUNCTION(proc_open)
13711371

13721372
if (newprocok == FALSE) {
13731373
DWORD dw = GetLastError();
1374-
close_all_descriptors(descriptors, ndesc);
13751374
char *msg = php_win32_error_to_msg(dw);
13761375
php_error_docref(NULL, E_WARNING, "CreateProcess failed: %s", msg);
13771376
php_win32_error_msg_free(msg);
@@ -1388,7 +1387,6 @@ PHP_FUNCTION(proc_open)
13881387

13891388
if (close_parentends_of_pipes(&factions, descriptors, ndesc) == FAILURE) {
13901389
posix_spawn_file_actions_destroy(&factions);
1391-
close_all_descriptors(descriptors, ndesc);
13921390
goto exit_fail;
13931391
}
13941392

@@ -1408,7 +1406,6 @@ PHP_FUNCTION(proc_open)
14081406
}
14091407
posix_spawn_file_actions_destroy(&factions);
14101408
if (r != 0) {
1411-
close_all_descriptors(descriptors, ndesc);
14121409
php_error_docref(NULL, E_WARNING, "posix_spawn() failed: %s", strerror(r));
14131410
goto exit_fail;
14141411
}
@@ -1450,7 +1447,6 @@ PHP_FUNCTION(proc_open)
14501447
_exit(127);
14511448
} else if (child < 0) {
14521449
/* Failed to fork() */
1453-
close_all_descriptors(descriptors, ndesc);
14541450
php_error_docref(NULL, E_WARNING, "Fork failed: %s", strerror(errno));
14551451
goto exit_fail;
14561452
}
@@ -1540,6 +1536,9 @@ PHP_FUNCTION(proc_open)
15401536
} else {
15411537
exit_fail:
15421538
_php_free_envp(env);
1539+
if (descriptors) {
1540+
close_all_descriptors(descriptors, ndesc);
1541+
}
15431542
RETVAL_FALSE;
15441543
}
15451544

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
proc_open() does not leak file descriptors when descriptor setup fails mid-spec
3+
--SKIPIF--
4+
<?php
5+
if (!function_exists("proc_open")) die("skip proc_open() unavailable");
6+
if (!@is_dir("/proc/self/fd")) die("skip requires /proc/self/fd");
7+
?>
8+
--FILE--
9+
<?php
10+
$before = count(scandir("/proc/self/fd"));
11+
for ($i = 0; $i < 100; $i++) {
12+
// Index 0 opens a real pipe; index 1 is invalid, so setup fails after the
13+
// pipe is already open. The aborted call must release the pipe fds.
14+
@proc_open("true", [0 => ["pipe", "r"], 1 => ["bogus_type"]], $pipes);
15+
}
16+
$after = count(scandir("/proc/self/fd"));
17+
var_dump($after <= $before + 2);
18+
?>
19+
--EXPECT--
20+
bool(true)

0 commit comments

Comments
 (0)