Skip to content

Commit b46fc8f

Browse files
committed
[linux] fix read_id_stat to handle unbalanced parens in process name
Fix issue #359 by changing the parse logic: find the first '(' and the last ')', the string between is the process name, the rest are the remaining fields. The old code tried to balance parentheses, which failed when the process name contained unbalanced parens (e.g. 'nc ('), causing lsof to silently skip the process. Also add a test case with a helper that sets its comm.
1 parent 1c3d6b4 commit b46fc8f

6 files changed

Lines changed: 157 additions & 72 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ lib/dialects/linux/tests/pipe
138138
lib/dialects/linux/tests/pty
139139
lib/dialects/linux/tests/ux
140140
lib/dialects/linux/tests/mmap
141+
lib/dialects/linux/tests/unbal_parens
141142

142143
# automake
143144
Makefile.in

Makefile.am

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ LINUX_TESTS = lib/dialects/linux/tests/case-10-mqueue.bash \
164164
lib/dialects/linux/tests/case-20-pipe-no-close-endpoint.bash \
165165
lib/dialects/linux/tests/case-20-pty-endpoint.bash \
166166
lib/dialects/linux/tests/case-20-ux-socket-endpoint.bash \
167-
lib/dialects/linux/tests/case-20-ux-socket-endpoint-unaccepted.bash
167+
lib/dialects/linux/tests/case-20-ux-socket-endpoint-unaccepted.bash \
168+
lib/dialects/linux/tests/case-20-unbal-parens.bash
168169
EXTRA_DIST += $(LINUX_TESTS) lib/dialects/linux/tests/Makefile lib/dialects/linux/tests/case-00-linux-hello.bash
169170
if LINUX
170171
check_PROGRAMS += lib/dialects/linux/tests/epoll \
@@ -176,7 +177,8 @@ check_PROGRAMS += lib/dialects/linux/tests/epoll \
176177
lib/dialects/linux/tests/pidfd \
177178
lib/dialects/linux/tests/pipe \
178179
lib/dialects/linux/tests/pty \
179-
lib/dialects/linux/tests/ux
180+
lib/dialects/linux/tests/ux \
181+
lib/dialects/linux/tests/unbal_parens
180182
lib_dialects_linux_tests_mq_fork_LDADD = -lrt
181183
lib_dialects_linux_tests_mq_open_LDADD = -lrt
182184

lib/dialects/linux/dproc.c

Lines changed: 59 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,120 +1666,109 @@ static int read_id_stat(struct lsof_context *ctx, /* context */
16661666
int *pgid) /* returned process group ID for PID
16671667
* type */
16681668
{
1669-
char buf[MAXPATHLEN], *cp, *cp1, **fp;
1670-
int ch, cx, es, pc;
1669+
char *cp, *cp1, *cp2, **fp;
1670+
int len = 0, n;
16711671
char *cbf = (char *)NULL;
1672-
MALLOC_S cbfa = 0;
1672+
int cbfa = 0;
16731673
FILE *fs;
1674+
int ret = 0;
16741675
/*
16751676
* Open the stat file path, assign a page size buffer to its stream,
1676-
* and read the file's first line.
1677+
* and read the entire file.
16771678
*/
16781679
if (!(fs = open_proc_stream(ctx, p, "r", 0)))
16791680
return (-1);
1680-
if (!(cp = fgets(buf, sizeof(buf), fs))) {
1681-
1682-
read_id_stat_exit:
1683-
1684-
CLEAN(cbf);
1685-
(void)fclose(fs);
1686-
return (-1);
1681+
for (;;) {
1682+
if (len + 1 >= cbfa) {
1683+
if (grow_array((void **)&cbf, &cbfa, sizeof(char), 256) != 0)
1684+
goto read_id_stat_exit;
1685+
}
1686+
n = fread(cbf + len, 1, cbfa - len - 1, fs);
1687+
if (n == 0) {
1688+
if (ferror(fs))
1689+
goto read_id_stat_exit;
1690+
break;
1691+
}
1692+
len += n;
16871693
}
1694+
if (!cbf || !*cbf)
1695+
goto read_id_stat_exit;
1696+
cbf[len] = '\0';
1697+
(void)fclose(fs);
1698+
fs = NULL;
16881699
/*
16891700
* Skip to the first field, and make sure it is a matching ID.
16901701
*/
1691-
cp1 = cp;
1702+
cp = cbf;
16921703
while (*cp && (*cp != ' ') && (*cp != '\t'))
16931704
cp++;
16941705
if (*cp)
16951706
*cp = '\0';
1696-
if (atoi(cp1) != id)
1707+
if (atoi(cbf) != id)
16971708
goto read_id_stat_exit;
16981709
/*
16991710
* The second field should contain the command, enclosed in parentheses.
1700-
* If it also has embedded '\n' characters, replace them with '?'
1701-
* characters, accumulating command characters until a closing parentheses
1702-
* appears.
1703-
*
1711+
* Find the first '(' and the last ')' in the file. The string between
1712+
* them is the command name; the rest after the last ')' are the
1713+
* remaining fields.
17041714
*/
17051715
for (++cp; *cp && (*cp == ' '); cp++)
17061716
;
17071717
if (!cp || (*cp != '('))
17081718
goto read_id_stat_exit;
1709-
cp++;
1710-
pc = 1; /* start the parenthesis balance count at 1 */
1711-
1719+
cp1 = ++cp;
1720+
cp2 = strrchr(cp, ')');
1721+
if (!cp2)
1722+
goto read_id_stat_exit;
17121723
/*
1713-
* Enter the command characters safely. Supply them from the initial read
1714-
* of the stat file line, a '\n' if the initial read didn't yield a ')'
1715-
* command closure, or by reading the rest of the command a character at
1716-
* a time from the stat file. Count embedded '(' characters and balance
1717-
* them with embedded ')' characters. The opening '(' starts the balance
1718-
* count at one.
1724+
* Replace embedded newlines in the command with '?'.
17191725
*/
1720-
for (cx = es = 0;;) {
1721-
if (!es)
1722-
ch = *cp++;
1723-
else {
1724-
if ((ch = fgetc(fs)) == EOF)
1725-
goto read_id_stat_exit;
1726-
}
1727-
if (ch == '(') /* a '(' advances the balance count */
1728-
pc++;
1729-
if (ch == ')') {
1730-
1731-
/*
1732-
* Balance parentheses when a closure is encountered. When
1733-
* they are balanced, this is the end of the command.
1734-
*/
1735-
pc--;
1736-
if (!pc)
1737-
break;
1738-
}
1739-
if ((cx + 2) > cbfa)
1740-
cbfa = alloc_cbf(ctx, (cx + 2), &cbf, cbfa);
1741-
cbf[cx] = ch;
1742-
cx++;
1743-
cbf[cx] = '\0';
1744-
if (!es && !*cp)
1745-
es = 1; /* Switch to fgetc() when a '\0' appears. */
1726+
for (cp = cp1; cp < cp2; cp++) {
1727+
if (*cp == '\n')
1728+
*cp = '?';
17461729
}
1747-
*cmd = cbf;
1748-
cbf = NULL;
1730+
*cp2 = '\0';
1731+
*cmd = mkstrcpy(cp1, NULL);
1732+
if (!*cmd)
1733+
goto read_id_stat_exit;
17491734
/*
1750-
* Read the remainder of the stat line if it was necessary to read command
1751-
* characters individually from the stat file.
1752-
*
1753-
* Separate the reminder into fields.
1735+
* Separate the remainder into fields.
17541736
*/
1755-
if (es)
1756-
cp = fgets(buf, sizeof(buf), fs);
1757-
(void)fclose(fs);
1737+
cp = cp2 + 1;
1738+
while (*cp && (*cp == ' '))
1739+
cp++;
17581740
if (!cp || !*cp)
1759-
return (-1);
1741+
goto read_id_stat_exit;
17601742
if (get_fields(ctx, cp, (char *)NULL, &fp, (int *)NULL, 0) < 3)
1761-
return (-1);
1743+
goto read_id_stat_exit;
17621744
/*
1763-
* Convert and return parent process (fourth field) and process group (fifth
1764-
* field) IDs.
1745+
* Convert and return parent process (fourth field) and process group
1746+
* (fifth field) IDs.
17651747
*/
17661748
if (fp[1] && *fp[1])
17671749
*ppid = atoi(fp[1]);
17681750
else
1769-
return (-1);
1751+
goto read_id_stat_exit;
17701752
if (fp[2] && *fp[2])
17711753
*pgid = atoi(fp[2]);
17721754
else
1773-
return (-1);
1755+
goto read_id_stat_exit;
17741756
/*
17751757
* Check the state in the third field. If it is 'Z', return that
17761758
* indication.
17771759
*/
17781760
if (fp[0] && !strcmp(fp[0], "Z"))
1779-
return (1);
1761+
ret = 1;
17801762
else if (fp[0] && !strcmp(fp[0], "T"))
1781-
return (2);
1782-
return (0);
1763+
ret = 2;
1764+
CLEAN(cbf);
1765+
return ret;
1766+
1767+
read_id_stat_exit:
1768+
CLEAN(cbf);
1769+
if (fs)
1770+
(void)fclose(fs);
1771+
return (-1);
17831772
}
17841773

17851774
/*

lib/dialects/linux/tests/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ HELPERS = \
88
pty \
99
ux \
1010
mmap \
11+
unbal_parens \
1112
\
1213
open_with_flags \
1314
\
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
#!/bin/bash
2+
source tests/common.bash
3+
4+
TARGET=$tcasedir/unbal_parens
5+
6+
r=0
7+
8+
test_pattern() {
9+
local pattern="$1"
10+
local expected="$2"
11+
local pid
12+
local output
13+
local comm
14+
15+
$TARGET "$pattern" | {
16+
read pid
17+
if [ -z "$pid" ]; then
18+
echo "FAIL: Failed to start helper process"
19+
exit 1
20+
fi
21+
22+
# Give /proc time to update
23+
sleep 0.2
24+
25+
# Check that lsof finds the process
26+
output=$($lsof -p $pid 2>/dev/null)
27+
if ! grep -Fq "$pid" <<<"$output"; then
28+
echo "FAIL: lsof did not find process with comm='$pattern'"
29+
echo "pid: $pid"
30+
kill $pid 2>/dev/null
31+
exit 1
32+
fi
33+
34+
# Check that the reported command name matches expected
35+
comm=$($lsof -p $pid -Fc 2>/dev/null | grep '^c' | head -1 | sed 's/^c//')
36+
if [ "$comm" != "$expected" ]; then
37+
echo "FAIL: comm mismatch for pattern='$pattern'"
38+
echo " expected: '$expected'"
39+
echo " got: '$comm'"
40+
kill $pid 2>/dev/null
41+
exit 1
42+
fi
43+
44+
echo "PASS: pattern='$pattern' comm='$comm'"
45+
kill $pid 2>/dev/null
46+
exit 0
47+
}
48+
49+
if [ $? -ne 0 ]; then
50+
r=1
51+
fi
52+
}
53+
54+
{
55+
# Test various unbalanced parenthesis patterns
56+
test_pattern "test(" "test(" || r=1
57+
test_pattern "test)" "test)" || r=1
58+
test_pattern "a(b)c" "a(b)c" || r=1
59+
test_pattern "a(b)c)d)e" "a(b)c)d)e" || r=1
60+
test_pattern "((" "((" || r=1
61+
test_pattern "))" "))" || r=1
62+
test_pattern "((()))" "((()))" || r=1
63+
test_pattern "(((" "(((" || r=1
64+
test_pattern ")))" ")))" || r=1
65+
66+
# Test newline in comm is replaced with '?'
67+
test_pattern "NEWLINE" "a?b" || r=1
68+
69+
exit $r
70+
} >> $report 2>&1
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#define _GNU_SOURCE
2+
#include <sys/prctl.h>
3+
#include <stdio.h>
4+
#include <string.h>
5+
#include <unistd.h>
6+
7+
int main(int argc, char **argv) {
8+
const char *name = (argc > 1) ? argv[1] : "test(";
9+
10+
if (strcmp(name, "NEWLINE") == 0) {
11+
name = "a\nb";
12+
}
13+
14+
if (prctl(PR_SET_NAME, name) < 0) {
15+
perror("prctl");
16+
return 1;
17+
}
18+
printf("%d\n", getpid());
19+
fflush(stdout);
20+
pause();
21+
return 0;
22+
}

0 commit comments

Comments
 (0)