okay, ignore that patch (not least because it was an unrelated old patch that's already been applied) and try this one instead... turns out we have two separate pidof bugs on Android. which is only fitting, because two separate bugs were raised today (though they were both for this latter bug, not the comm bug i found myself while investigating).
[PATCH] Fix the pidof comm and cmdline tests. If we have a 15-byte name, we don't know whether comm actually matches or is a truncated form of a longer name that has a common prefix. For example, with "this-is-a-very-long-name-that-is-too-long", we shouldn't match "this-is-a-very-" (but the old code would). The cmdline code was also broken on Android because it used basename(3) rather than getbasename. This doesn't affect glibc because there's a workaround in portability.h to ensure that we get the non-POSIX basename(3) with glibc but then a non-glibc section that ensures everyone else gets POSIX basename(3). That should probably be removed (and maybe `basename` poisoned) to prevent similar mistakes in future. Bug: http://b/73123244 --- lib/lib.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) On Thu, Feb 8, 2018 at 10:44 PM, enh <e...@google.com> wrote: > If we have a 15-byte name, we don't know whether comm actually matches > or is a truncated form of a longer name that has a common prefix. > > For example, with "this-is-a-very-long-name-that-is-too-long", we shouldn't > match "this-is-a-very-" (but the old code would). > > Bug: http://b/73123244 > --- > lib/lib.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-)
From 4320baa048178e7cc36f471b1f6de53cffd636f6 Mon Sep 17 00:00:00 2001 From: Elliott Hughes <e...@google.com> Date: Thu, 8 Feb 2018 22:41:52 -0800 Subject: [PATCH] Fix the pidof comm and cmdline tests. If we have a 15-byte name, we don't know whether comm actually matches or is a truncated form of a longer name that has a common prefix. For example, with "this-is-a-very-long-name-that-is-too-long", we shouldn't match "this-is-a-very-" (but the old code would). The cmdline code was also broken on Android because it used basename(3) rather than getbasename. This doesn't affect glibc because there's a workaround in portability.h to ensure that we get the non-POSIX basename(3) with glibc but then a non-glibc section that ensures everyone else gets POSIX basename(3). That should probably be removed (and maybe `basename` poisoned) to prevent similar mistakes in future. Bug: http://b/73123244 --- lib/lib.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/lib.c b/lib/lib.c index 7f5fbbd..e5c9479 100644 --- a/lib/lib.c +++ b/lib/lib.c @@ -1036,13 +1036,13 @@ void names_to_pid(char **names, int (*callback)(pid_t pid, char *name)) for (cur = names; *cur; cur++) { struct stat st1, st2; - char *bb = basename(*cur); - off_t len; + char *bb = getbasename(*cur); + off_t len = strlen(bb); - // fast path: only matching a filename (no path) that fits in comm - if (strncmp(comm, bb, 15)) continue; - len = strlen(bb); - if (bb==*cur && len<16) goto match; + // Fast path: only matching a filename (no path) that fits in comm. + // `len` must be 14 or less because with a full 15 bytes we don't + // know whether the name fit or was truncated. + if (len<=14 && bb==*cur && !strcmp(comm, bb)) goto match; // If we have a path to existing file only match if same inode if (bb!=*cur && !stat(*cur, &st1)) { @@ -1059,12 +1059,12 @@ void names_to_pid(char **names, int (*callback)(pid_t pid, char *name)) sprintf(cmd = libbuf+16, "/proc/%u/cmdline", u); len = sizeof(libbuf)-17; if (!(cmd = readfileat(AT_FDCWD, cmd, cmd, &len))) continue; - // readfile only guarnatees one null terminator and we need two + // readfile only guarantees one null terminator and we need two // (yes the kernel should do this for us, don't care) cmd[len] = 0; } - if (!strcmp(bb, basename(cmd))) goto match; - if (bb!=*cur && !strcmp(bb, basename(cmd+strlen(cmd)+1))) goto match; + if (!strcmp(bb, getbasename(cmd))) goto match; + if (bb!=*cur && !strcmp(bb, getbasename(cmd+strlen(cmd)+1))) goto match; continue; match: if (callback(u, *cur)) break; -- 2.16.0.rc1.238.g530d649a79-goog
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
