On Tue, 28 Jan 2025, Kang-Che Sung wrote:
Perhaps what you really intended is this?
const char *passwd_argv[] = { "passwd", "--", login_name, NULL };
That would be much more intuitive, readable, safer and so on, but then
you'll need a cast when pass it to the pre-const functions,
eg (char **)passwd_argv.
From the surrounding code I've read, you are attempting to pass a
".rodata" string into an "argv" character pointer array.
But "argv" is supposed to be mutable by the callee (i.e. the new
program's main function). (I think it's required by the C standard.)
Unless there's a guarantee that the callee never modifies the "argv"
string, passing ".rodata" strings with de-const casts would be unsafe.
That's why when I initially saw the patch and code I was nervous. (A
(char *) de-const cast without an in-code explanation of why is a red
flag to me.)
Ah, you're right. I was focused on the fact that the consumer of the
char** argv was
int execvp(const char *file, char *const argv[]);
where I hoped, as a syscall, it would copy-out the argv[] in a read-only
way which makes the cast safe.
But, as you point out, in this patchset run_applet_no_and_exit() will
be passing a (casted) rodata argv straight to an applet's main().
To avoid unnecessary heap allocations and copying on tiny targets,
(by str*dup'ing argv[] every time) maybe we can add const as another
restriction required by NOEXEC. I mean we might find we're lucky that most
of the ~167 NOEXEC applets could already change their main signature from
(int argc, char **argv)
to
(int argc, const char *const *argv)
and for those that can't, ie those that write to argv[], we may
find they can be easily fixed to use a local copy instead.
_______________________________________________
busybox mailing list
[email protected]
https://lists.busybox.net/mailman/listinfo/busybox
