Skip to content

Commit

Permalink
pspax: rework & document get_proc_name
Browse files Browse the repository at this point in the history
The current scanf format tries to use "%s.16" to limit reading to 16
bytes, but that doesn't actually work -- the maximum field width is
between the "%" and the "s", so it should have been "%16s".  This ends
up working anyways because the %s consumes the entire string before it
stops, and then scanf stops processing after it can't match ".16".  If
the size of the field were BUFSIZE or larger, then it'd overflow.  In
practice, BUFSIZ tends to be "large" (i.e. O(KiB)), and the kernel will
truncate this field to 16 bytes for userspace programs.  Kernel threads
can have longer names, but not that big.  At least, on Linux.

Fix the scanf string to properly limit to 15 bytes, and change the local
buffer to be exactly 16 bytes rather than the unrelated BUFSIZ (which is
a stdio.h buffer size, and nothing related to kernel processes).  Then
add some more comments to explain what the code is actually doing, and
simplify the final NUL logic to avoid redundant work.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
  • Loading branch information
vapier committed Jan 24, 2024
1 parent 063a906 commit 1cf2124
Showing 1 changed file with 23 additions and 5 deletions.
28 changes: 23 additions & 5 deletions pspax.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,13 @@ static const char *get_proc_name_cmdline(int pfd)
static const char *get_proc_name(int pfd)
{
FILE *fp;
static char str[BUFSIZ];
/*
* The stat file says process names are truncated to TASK_COMM_LEN (16) bytes.
* That includes the trailing NUL (\0) byte. This is true for userspace, but
* kernel processes seem to be unlimited. We don't care about those in this
* program though, so truncating them all the time is fine.
*/
static char str[16];

if (wide_output)
return get_proc_name_cmdline(pfd);
Expand All @@ -105,18 +111,30 @@ static const char *get_proc_name(int pfd)
if (fp == NULL)
return NULL;

if (fscanf(fp, "%*d %s.16", str) != 1) {
/*
* The format is:
* <pid> (<name>) ...more fields...
* For example:
* 1234 (bash) R ...
*
* Match the leading (, then read 15 bytes (since scanf writes, but doesn't count,
* NUL bytes, so it will write up to 16 bytes to str). Ignore the rest rather than
* look for closing ) since kernel processes can be longer.
*/
if (fscanf(fp, "%*d (%15s", str) != 1) {
fclose(fp);
return NULL;
}

if (*str) {
str[strlen(str) - 1] = '\0';
str[16] = 0;
/* Discard trailing ) if it exists. */
size_t len = strlen(str);
if (str[len - 1] == ')')
str[len - 1] = '\0';
}
fclose(fp);

return (str+1);
return str;
}

static int get_proc_maps(int pfd)
Expand Down

0 comments on commit 1cf2124

Please sign in to comment.