busybox: fix busybox lock applet pidstr buffer overflow #9499
Conversation
|
Did you send this to upstream? |
| @@ -74,7 +74,7 @@ | |||
| +{ | |||
| + int pid; | |||
| + int flags; | |||
| + char pidstr[8]; | |||
| + char pidstr[10]; | |||
There was a problem hiding this comment.
I would prefer 11 here to also store the termination NULL byte.
INT_MAX is 2147483647
There was a problem hiding this comment.
Please also change the place where this buffer is used to
snprintf(sizeof(pidstr), pidstr, "%d\n", pid);
| @@ -74,7 +74,7 @@ | |||
| +{ | |||
| + int pid; | |||
There was a problem hiding this comment.
Please change this from int to pid_t
Kernel setting `/proc/sys/kernel/pid_max` can be set up to 4194304(7 digits) which will cause buffer overflow in busbox lock patch, this often happens when running in a rootfs container environment. This commit enlarges `pidstr` to 12 bytes to ensure a sufficient buffer for pid number and an additional char '\n'. Signed-off-by: Qichao Zhang <njuzhangqichao@gmail.com>
usbuild
force-pushed
the
bugfix_busybox_lock_pid_length
branch
from
b95885a
to
af5031d
Compare
Thanks for your advises . I haven't found the upstream address for this lock util file, it seems be used in OpenWRT only. |
|
So upstream busybox doesn't support locking? I'd try to get the whole thing merged into upstream then |
|
Thank you for the patch, I applied it t mater in 3456775. |
|
I added an additional fix in d80336e. |
Kernel setting
/proc/sys/kernel/pid_maxcan be set up to 4194304(7 digits) which will cause buffer overflow in busbox lock patch, this often happens when running in a rootfs container environment.This commit enlarges
pidstrto 10 bytes to ensure a sufficient buffer for pid number and an additional char '\n'.