Bug 12296

Summary: pidof fails to find tasks with names 15 characters long
Product: Busybox Reporter: Alin Nastac <alin.nastac>
Component: OtherAssignee: unassigned
Status: NEW ---    
Severity: major CC: busybox-cvs
Priority: P5    
Version: 1.31.x   
Target Milestone: ---   
Hardware: All   
OS: Linux   
Host: Target:
Build:
Attachments: find_pid_by_name() fix

Description Alin Nastac 2019-10-30 12:17:40 UTC
Created attachment 8276 [details]
find_pid_by_name() fix

find_pid_by_name() first compare procName with p->comm and returns true when they're matched, but only when both following conditions are met:
  strncmp(p->comm, procName, 15) == 0
  strlen(p->comm) <= 14

However when strlen(p->comm) is 15, this function will compare procName with p->argv0 and return false when p->argv0 is NULL.

When argv0 support is not compiled in, find_pid_by_name() should assume that p->comm name has not been truncated.
Comment 1 Denys Vlasenko 2019-10-31 15:05:28 UTC
This will cause false positives - matching unrelated processes whose names match first 15 chars. I bet people wouldn't be happy about that.
Comment 2 Alin Nastac 2019-10-31 16:15:52 UTC
I'm not very happy that pidof fails to find the PID I'm looking for just because its name happens to be 15 characters long.

What do you think is more important?
a) 0.0001% chance of having a false positive
b) certainty of having false negatives when looking for processes with 15 characters in their name
Comment 3 Denys Vlasenko 2019-11-01 13:24:52 UTC
(In reply to Alin Nastac from comment #2)
> What do you think is more important?
> a) 0.0001% chance of having a false positive

Certainly this one: people will get mad when they would kill an unrelated process. What excuse would you propose I can use for doing that?

BTW.

> When argv0 support is not compiled in

...which should not be possible:

        PSSCAN_ARGVN    = (1 << 16) * (ENABLE_KILLALL
                                || ENABLE_PGREP || ENABLE_PKILL
                                || ENABLE_PIDOF
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                || ENABLE_SESTATUS

Can you debug why this part:

                /* or we require argv0 to match (essential for matching reexeced /proc/self/exe)*/
                 || (p->argv0 && strcmp(bb_basename(p->argv0), procName) == 0)

does not work in your case?
Comment 4 Alin Nastac 2019-11-04 11:45:14 UTC
I have no idea why argv0 is NULL, but that's what I see. If you say it shouldn't happen, then my patch should not cause any problem for you.

I tried to compile busybox with debug information so I could find out why argv0 is NULL, but compiler refuse to generate debug information although I've added -g3 and -ggdb3 to the command line:
  arm-openwrt-linux-gnueabi-gcc -Wp,-MD,libbb/.procps.o.d   -std=gnu99 -Iinclude -Ilibbb  -include include/autoconf.h -D_GNU_SOURCE -DNDEBUG -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D"BB_VER=KBUILD_STR(1.31.0)"  -Wall -Wshadow -Wwrite-strings -Wundef -Wstrict-prototypes -Wunused -Wunused-parameter -Wunused-function -Wunused-value -Wmissing-prototypes -Wmissing-declarations -Wno-format-security -Wdeclaration-after-statement -Wold-style-definition -fno-builtin-strlen -finline-limit=0 -fomit-frame-pointer -ffunction-sections -fdata-sections -fno-guess-branch-probability -funsigned-char -falign-functions=1 -falign-jumps=1 -falign-labels=1 -falign-loops=1 -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-builtin-printf -Os  -Os -pipe -mtune=cortex-a7 -fno-caller-saves -Wno-error=unused-but-set-variable -g3 -fno-caller-saves -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -mfloat-abi=soft -iremap/usr/localdisk2/vcnt-j/openwrt/build_dir/target-arm_cortex-a7_glibc_eabi/busybox-1.31.0:busybox-1.31.0 -D_FORTIFY_SOURCE=2 -Wl,-z,now -Wl,-z,relro -flto -O0 -g3 -ggdb3 -I/usr/localdisk2/vcnt-j/openwrt/staging_dir/target-arm_cortex-a7_glibc_eabi/usr/include -I/usr/localdisk2/vcnt-j/openwrt/staging_dir/target-arm_cortex-a7_glibc_eabi/include -I/usr/localdisk2/vcnt-j/openwrt/staging_dir/toolchain-arm_cortex-a7_gcc-5.5.0_glibc_eabi/usr/include -I/usr/localdisk2/vcnt-j/openwrt/staging_dir/toolchain-arm_cortex-a7_gcc-5.5.0_glibc_eabi/include   -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(procps)"  -D"KBUILD_MODNAME=KBUILD_STR(procps)" -c -o libbb/procps.o libbb/procps.c

No matter what I do, gdb says that resulted libbb/procps.o contain no debug information.