Call prctl(2) with long integers, specify 5 arguments, and avoid casts#3085
Call prctl(2) with long integers, specify 5 arguments, and avoid casts#3085karelzak wants to merge 5 commits into
Conversation
|
I will send a smaller v2 patch with just the change from 0 to 0L and removing the casts. I'll keep the short calls without the 0L, as @t-8ch pointed out. |
You can also create PR on github :-) |
I personally dislike the centralizing that GH represents for free software projects, and the lock-in that many projects have chosen for staying on GH. Also, I find git-send-email(1) much simpler than having to set up a GH fork, and accessing a webUI interface. On the other hand, I understand that it might be simpler for you as a reviewer, since it runs CI automatically for you. If you want me to open a PR, I can do it. By default, I prefer a mailing list. :-) |
|
Mailing list is absolutely OK :-) |
Add a new ul_default_shell() function to provide consistent shell resolution across util-linux tools. The function follows a priority order: $SHELL environment variable, user's shell from passwd database, and finally _PATH_BSHELL as fallback. The function supports flags to control its behavior: - UL_SHELL_NOENV: skip $SHELL environment variable check - UL_SHELL_NOPWD: skip passwd database lookup This addresses the issue where tools like script(1) would default to /bin/sh without respecting the user's configured shell, potentially causing data loss. Addresses: util-linux#3865 Suggested-by: Karel Zak <kzak@redhat.com> Suggested-by: Thomas Weißschuh <thomas@t-8ch.de> Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
Update tools that spawn interactive shells to use ul_default_shell() for consistent shell resolution. This ensures these tools respect both $SHELL and the user's configured shell from the passwd database before falling back to _PATH_BSHELL. Affected tools: - script(1): fixes history truncation when invoked without $SHELL - scriptlive(1): consistent with script(1) behavior - flock(1): for -c command execution - more(1): for shell escape feature - exec_shell (used by unshare(1) and nsenter(1)) This change addresses user reports of data loss due to tools defaulting to /bin/sh instead of the user's configured shell, particularly affecting command history with different HISTSIZE configurations. Addresses: util-linux#3865 Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
Remove local DEFAULT_SHELL definitions and hardcoded "/bin/sh" strings in favor of the standard _PATH_BSHELL macro from <paths.h>. This provides consistency across the codebase while following libc conventions. These tools already perform their own passwd lookups and only need a fallback value, so they don't require the full ul_default_shell() resolution logic. Affected tools: - su(1): already checks pw_shell validity - sulogin(8): emergency login with explicit shell handling - setpriv(1): already has passwd entry for environment setup Signed-off-by: Alessandro Ratti <alessandro@0x65c.net>
* 'fix/issue_3865' of https://github.com/lord2y/util-linux: login-utils, sys-utils: use _PATH_BSHELL consistently *: use ul_default_shell() for interactive shell spawning lib: introduce ul_default_shell() for consistent shell resolution
|
@alejandro-colomar, sorry, I forgot this issue/PR What is the current state of this? Would you like me to send a new version? |
I also forgot. :) I'll have a look at the patches and see if it needs any rebasing. Thanks for the reminder! |
Since libc's prctl(2) wrapper is a variadic function, arguments must have the right width. Otherwise, the behavior is undefined. The kernel expects long arguments, so let's provide them. Also, avoid some casts to unsigned long, by changing the type of the parameter in some local wrappers. And use consistently 0L. 0UL is basically the same, and uses one more character. Keep it short. Also, unsigned integer literals are dangerous, as the compiler can't diagnose mistakes such as overflow. - Casts are evil. - prctl(2) expects long integers, and Link: <https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6698b096a6f5342cb9b338c237ed875a8635497a> Link: <https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954> Link: <https://lore.kernel.org/util-linux/20240601093150.16912-1-alx@kernel.org/T/#u> Link: <util-linux#3085> Cc: Xi Ruoyao <xry111@xry111.site> Cc: Thomas Weißschuh <thomas@t-8ch.de> Cc: Karel Zak <kzak@redhat.com> Signed-off-by: Alejandro Colomar <alx@kernel.org>
828ea38 to
6ed54a5
Compare
Since libc's prctl(2) wrapper is a variadic function, arguments must have the right width. Otherwise, the behavior is undefined.
Also, the 5 arguments must be specified always, or the behavior is also undefined. libc reads 5 values and passes them all to the kernel, so if one is uninitialized, the kernel will receive garbagge, which could result in EINVAL (most likely), or worse, a different action.
Also, avoid some casts to unsigned long, by changing the type of the parameter to some local wrappers.
And use consistently 0L. 0UL is basically the same, and uses one more character. Keep it short.
Link: https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=6698b096a6f5342cb9b338c237ed875a8635497a
Link: https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954
Cc: Xi Ruoyao xry111@xry111.site