Skip to content

Commit 4aa1bf9

Browse files
JoshuaBehrensbukka
JoshuaBehrens
authored and
bukka
committed
Warn when fpm socket was not registered on the expected path
This might happen if the UDS length limit is exceeded. Co-authored-by: Jakub Zelenka <[email protected]> Closes GH-11066
1 parent 72e2e25 commit 4aa1bf9

6 files changed

+209
-9
lines changed

‎sapi/fpm/fpm/fpm_sockets.c

+17-1
Original file line numberDiff line numberDiff line change
@@ -396,9 +396,25 @@ static int fpm_socket_af_inet_listening_socket(struct fpm_worker_pool_s *wp) /*
396396
static int fpm_socket_af_unix_listening_socket(struct fpm_worker_pool_s *wp) /* {{{ */
397397
{
398398
struct sockaddr_un sa_un;
399+
size_t socket_length = sizeof(sa_un.sun_path);
400+
size_t address_length = strlen(wp->config->listen_address);
399401

400402
memset(&sa_un, 0, sizeof(sa_un));
401-
strlcpy(sa_un.sun_path, wp->config->listen_address, sizeof(sa_un.sun_path));
403+
strlcpy(sa_un.sun_path, wp->config->listen_address, socket_length);
404+
405+
if (address_length >= socket_length) {
406+
zlog(
407+
ZLOG_WARNING,
408+
"[pool %s] cannot bind to UNIX socket '%s' as path is too long (found length: %zu, "
409+
"maximal length: %zu), trying cut socket path instead '%s'",
410+
wp->config->name,
411+
wp->config->listen_address,
412+
address_length,
413+
socket_length,
414+
sa_un.sun_path
415+
);
416+
}
417+
402418
sa_un.sun_family = AF_UNIX;
403419
return fpm_sockets_get_listening_socket(wp, (struct sockaddr *) &sa_un, sizeof(struct sockaddr_un));
404420
}

‎sapi/fpm/fpm/fpm_unix.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <stdlib.h>
99
#include <unistd.h>
1010
#include <sys/types.h>
11+
#include <sys/un.h>
1112
#include <pwd.h>
1213
#include <grp.h>
1314

@@ -63,6 +64,33 @@ static struct passwd *fpm_unix_get_passwd(struct fpm_worker_pool_s *wp, const ch
6364
return pwd;
6465
}
6566

67+
static inline bool fpm_unix_check_listen_address(struct fpm_worker_pool_s *wp, const char *address, int flags)
68+
{
69+
if (wp->listen_address_domain != FPM_AF_UNIX) {
70+
return true;
71+
}
72+
73+
struct sockaddr_un test_socket;
74+
size_t address_length = strlen(address);
75+
size_t socket_length = sizeof(test_socket.sun_path);
76+
77+
if (address_length < socket_length) {
78+
return true;
79+
}
80+
81+
zlog(
82+
flags,
83+
"[pool %s] cannot bind to UNIX socket '%s' as path is too long (found length: %zu, "
84+
"maximal length: %zu)",
85+
wp->config->name,
86+
address,
87+
address_length,
88+
socket_length
89+
);
90+
91+
return false;
92+
}
93+
6694
static inline bool fpm_unix_check_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
6795
{
6896
return !name || fpm_unix_is_id(name) || fpm_unix_get_passwd(wp, name, flags);
@@ -90,6 +118,7 @@ bool fpm_unix_test_config(struct fpm_worker_pool_s *wp)
90118
return (
91119
fpm_unix_check_passwd(wp, config->user, ZLOG_ERROR) &&
92120
fpm_unix_check_group(wp, config->group, ZLOG_ERROR) &&
121+
fpm_unix_check_listen_address(wp, config->listen_address, ZLOG_SYSERROR) &&
93122
fpm_unix_check_passwd(wp, config->listen_owner, ZLOG_SYSERROR) &&
94123
fpm_unix_check_group(wp, config->listen_group, ZLOG_SYSERROR)
95124
);
@@ -273,7 +302,7 @@ int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *pa
273302
/* Copy the new ACL entry from config */
274303
for (i=ACL_FIRST_ENTRY ; acl_get_entry(aclconf, i, &entryconf) ; i=ACL_NEXT_ENTRY) {
275304
if (0 > acl_create_entry (&aclfile, &entryfile) ||
276-
0 > acl_copy_entry(entryfile, entryconf)) {
305+
0 > acl_copy_entry(entryfile, entryconf)) {
277306
zlog(ZLOG_SYSERROR, "[pool %s] failed to add entry to the ACL of the socket '%s'", wp->config->name, path);
278307
acl_free(aclfile);
279308
return -1;

‎sapi/fpm/tests/logreader.inc

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class LogReader
1616
*
1717
* @var string|null
1818
*/
19-
private ?string $currentSourceName;
19+
private ?string $currentSourceName = null;
2020

2121
/**
2222
* Log descriptors.

‎sapi/fpm/tests/socket-uds-too-long-filename-start.phpt

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
--TEST--
2+
FPM: UNIX socket filename is too for start
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc"; ?>
6+
--FILE--
7+
<?php
8+
9+
require_once "tester.inc";
10+
$socketFilePrefix = __DIR__ . '/socket-file';
11+
$socketFile = sprintf(
12+
"%s-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
13+
$socketFilePrefix,
14+
str_repeat('-0000', 11)
15+
);
16+
17+
$cfg = <<<EOT
18+
[global]
19+
error_log = {{FILE:LOG}}
20+
21+
[fpm_pool]
22+
listen = $socketFile
23+
pm = static
24+
pm.max_children = 1
25+
catch_workers_output = yes
26+
EOT;
27+
28+
$tester = new FPM\Tester($cfg);
29+
$tester->start();
30+
$tester->expectLogStartNotices();
31+
$tester->expectLogPattern(
32+
sprintf(
33+
'/\[pool fpm_pool\] cannot bind to UNIX socket \'%s\' as path is too long '
34+
. '\(found length: %d, maximal length: \d+\), trying cut socket path instead \'.+\'/',
35+
preg_quote($socketFile, '/'),
36+
strlen($socketFile)
37+
),
38+
true
39+
);
40+
41+
$files = glob($socketFilePrefix . '*');
42+
43+
if ($files === []) {
44+
echo 'Socket files were not found.' . PHP_EOL;
45+
}
46+
47+
if ($socketFile === $files[0]) {
48+
// this means the socket file path length is not an issue (anymore). Might be not long enough
49+
echo 'Socket file is the same as configured.' . PHP_EOL;
50+
}
51+
52+
$tester->terminate();
53+
$tester->expectLogTerminatingNotices();
54+
$tester->close();
55+
?>
56+
Done
57+
--EXPECT--
58+
Done
59+
--CLEAN--
60+
<?php
61+
require_once "tester.inc";
62+
FPM\Tester::clean();
63+
64+
// cleanup socket file if php-fpm was not killed
65+
$socketFile = sprintf(
66+
"/socket-file-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
67+
__DIR__,
68+
str_repeat('-0000', 11)
69+
);
70+
71+
if (is_file($socketFile)) {
72+
unlink($socketFile);
73+
}
74+
?>

‎sapi/fpm/tests/socket-uds-too-long-filename-test.phpt

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
FPM: UNIX socket filename is too for test
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc"; ?>
6+
--FILE--
7+
<?php
8+
9+
require_once "tester.inc";
10+
11+
$socketFilePrefix = __DIR__ . '/socket-file';
12+
$socketFile = sprintf(
13+
"/socket-file-fpm-unix-socket-too-long-filename-but-starts-anyway%s.sock",
14+
__DIR__,
15+
str_repeat('-0000', 11)
16+
);
17+
18+
$cfg = <<<EOT
19+
[global]
20+
error_log = {{FILE:LOG}}
21+
22+
[fpm_pool]
23+
listen = $socketFile
24+
pm = static
25+
pm.max_children = 1
26+
catch_workers_output = yes
27+
EOT;
28+
29+
$tester = new FPM\Tester($cfg);
30+
$tester->testConfig(true, [
31+
sprintf(
32+
'/cannot bind to UNIX socket \'%s\' as path is too long '
33+
. '\(found length: %d, maximal length: \d+\)/',
34+
preg_quote($socketFile, '/'),
35+
strlen($socketFile)
36+
),
37+
'/FPM initialization failed/',
38+
]);
39+
?>
40+
Done
41+
--EXPECT--
42+
Done
43+
--CLEAN--
44+
<?php
45+
require_once "tester.inc";
46+
FPM\Tester::clean();
47+
?>

‎sapi/fpm/tests/tester.inc

+40-6
Original file line numberDiff line numberDiff line change
@@ -382,26 +382,50 @@ class Tester
382382
* @return null|array
383383
* @throws \Exception
384384
*/
385-
public function testConfig($silent = false)
385+
public function testConfig($silent = false, array|string|null $expectedPattern = null): ?array
386386
{
387387
$configFile = $this->createConfig();
388388
$cmd = self::findExecutable() . ' -n -tt -y ' . $configFile . ' 2>&1';
389389
$this->trace('Testing config using command', $cmd, true);
390390
exec($cmd, $output, $code);
391+
$found = 0;
392+
if ($expectedPattern !== null) {
393+
$expectedPatterns = is_array($expectedPattern) ? $expectedPattern : [$expectedPattern];
394+
}
391395
if ($code) {
392396
$messages = [];
393397
foreach ($output as $outputLine) {
394398
$message = preg_replace("/\[.+?\]/", "", $outputLine, 1);
399+
if ($expectedPattern !== null) {
400+
for ($i = 0; $i < count($expectedPatterns); $i++) {
401+
$pattern = $expectedPatterns[$i];
402+
if ($pattern !== null && preg_match($pattern, $message)) {
403+
$found++;
404+
$expectedPatterns[$i] = null;
405+
}
406+
}
407+
}
395408
$messages[] = $message;
396409
if ( ! $silent) {
397410
$this->error($message, null, false);
398411
}
399412
}
413+
} else {
414+
$messages = null;
415+
}
400416

401-
return $messages;
417+
if ($expectedPattern !== null && $found < count($expectedPatterns)) {
418+
$missingPatterns = array_filter($expectedPatterns);
419+
$errorMessage = sprintf(
420+
"The expected config %s %s %s not been found",
421+
count($missingPatterns) > 1 ? 'patterns' : 'pattern',
422+
implode(', ', $missingPatterns),
423+
count($missingPatterns) > 1 ? 'have' : 'has',
424+
);
425+
$this->error($errorMessage);
402426
}
403427

404-
return null;
428+
return $messages;
405429
}
406430

407431
/**
@@ -1155,9 +1179,19 @@ class Tester
11551179
return $address;
11561180
}
11571181

1158-
return sys_get_temp_dir() . '/' .
1159-
hash('crc32', dirname($address)) . '-' .
1160-
basename($address);
1182+
$addressPart = hash('crc32', dirname($address)) . '-' . basename($address);
1183+
1184+
// is longer on Mac, than on Linux
1185+
$tmpDirAddress = sys_get_temp_dir() . '/' . $addressPart;
1186+
;
1187+
1188+
if (strlen($tmpDirAddress) <= 104) {
1189+
return $tmpDirAddress;
1190+
}
1191+
1192+
$srcRootAddress = dirname(__DIR__, 3) . '/' . $addressPart;
1193+
1194+
return $srcRootAddress;
11611195
}
11621196

11631197
return $this->getHost($type) . ':' . $port;

0 commit comments

Comments
 (0)