[PATCH v3 04/27] adduser: replace BB_EXECLP call with BB_EXECVP

[d+busybox at adaptive-enterprises.com]

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.