On Friday 04 March 2011 00:45, Dudy Kohen wrote:
> Hi,
> A lot of "aftermarket" Android developers add busybox to their builds.
> I have found out that the hardcoded default login shell is not valid
> in Android and it causes issues with scrips that do not have shebangs.
> I have sent this to one of the AOSP-based projects and it got
> accepted, and I thought it would benefit if it existed upstream as
> well.
> I know that you do not have Android code as part of your normal
> source, but this change is controlled by #ifdefs and thus should not
> affect the resulting code at all if not defined as Android.
> Thanks,
> David Kohen
Rob's suggestion to make LIBBB_DEFAULT_LOGIN_SHELL and DEFAULT_SHELL_SHORT_NAME
configurable sounds better to me.
Moreover, an even better thing we can do here is to use this problem
as an opportunity to audit and fix the code so that shell name
can be overridden with $SHELL variable (where applicable: say, su
shouldn't use it), and by user's default shell.
On android, is $SHELL set correctly? Do users have pw->pw_shell
which point to correct shell?
Let's look at the current code.
$ grep -B3 -r DEFAULT_SHELL .
./loginutils/su.c- * is a username that is retrieved via NIS (YP), that
doesn't have
./loginutils/su.c- * a default shell listed. */
./loginutils/su.c- if (!pw->pw_shell || !pw->pw_shell[0])
./loginutils/su.c: pw->pw_shell = (char *)DEFAULT_SHELL;
--
Seems to be ok as-is: normally pw->pw_shell will have correct value.
./loginutils/adduser.c- }
./loginutils/adduser.c-
./loginutils/adduser.c- pw.pw_gecos = (char *)"Linux User,,,";
./loginutils/adduser.c: pw.pw_shell = (char *)DEFAULT_SHELL;
--
I doubt you'll want to use adduser on Android
./loginutils/login.c- change_identity(pw);
./loginutils/login.c- shell = pw->pw_shell;
./loginutils/login.c- if (!shell || !shell[0])
./loginutils/login.c: shell = DEFAULT_SHELL;
--
Seems to be ok as-is: normally pw->pw_shell will have correct value.
./util-linux/script.c- }
./util-linux/script.c- shell = getenv("SHELL");
./util-linux/script.c- if (shell == NULL) {
./util-linux/script.c: shell = DEFAULT_SHELL;
--
Should we try to retrieve current user's shell (getpwuid(getuid())->pw_shell)
before falling back to DEFAULT_SHELL?
./miscutils/crond.c- xsetenv("HOME", pas->pw_dir);
./miscutils/crond.c-#endif
./miscutils/crond.c- /* currently, we use constant one: */
./miscutils/crond.c: /*setenv("SHELL", DEFAULT_SHELL, 1); - done earlier */
--
./miscutils/crond.c- }
./miscutils/crond.c- }
./miscutils/crond.c-
./miscutils/crond.c: line->cl_pid = fork_job(user, mailFd, DEFAULT_SHELL,
line->cl_cmd);
--
./miscutils/crond.c- /* initgroups, setgid, setuid, and chdir to
home or TMPDIR */
./miscutils/crond.c- change_user(pas);
./miscutils/crond.c- if (DebugOpt) {
./miscutils/crond.c: crondlog(LVL5 "child running %s",
DEFAULT_SHELL);
./miscutils/crond.c- }
./miscutils/crond.c- /* crond 3.0pl1-100 puts tasks in separate
process groups */
./miscutils/crond.c- bb_setpgrp();
./miscutils/crond.c: execl(DEFAULT_SHELL, DEFAULT_SHELL, "-c",
line->cl_cmd, (char *) NULL);
./miscutils/crond.c: crondlog(ERR20 "can't execute '%s' for user
%s", DEFAULT_SHELL, user);
--
./miscutils/crond.c-
./miscutils/crond.c- xchdir(G.crontab_dir_name);
./miscutils/crond.c- //signal(SIGHUP, SIG_IGN); /* ? original crond dies on
HUP... */
./miscutils/crond.c: xsetenv("SHELL", DEFAULT_SHELL); /* once, for all
future children */
--
Should we try to use $SHELL, then try retrieve user's shell
before falling back to DEFAULT_SHELL?
./miscutils/crontab.c- /* CHILD - change user and run editor */
./miscutils/crontab.c- /* initgroups, setgid, setuid */
./miscutils/crontab.c- change_identity(pas);
./miscutils/crontab.c: setup_environment(DEFAULT_SHELL,
--
Should we try to use $SHELL, then try retrieve user's shell
before falling back to DEFAULT_SHELL?
./miscutils/conspy.c- char *shell = getenv("SHELL");
./miscutils/conspy.c-
./miscutils/conspy.c- if (!shell)
./miscutils/conspy.c: shell = (char *) DEFAULT_SHELL;
--
Should we try to retrieve current user's shell (getpwuid(getuid())->pw_shell)
before falling back to DEFAULT_SHELL?
./libbb/run_shell.c-
./libbb/run_shell.c-#endif
./libbb/run_shell.c-
./libbb/run_shell.c:/* Run SHELL, or DEFAULT_SHELL if SHELL is "" or NULL.
--
./libbb/run_shell.c- args = xmalloc(sizeof(char*) * (4 +
additional_args_cnt));
./libbb/run_shell.c-
./libbb/run_shell.c- if (!shell || !shell[0])
./libbb/run_shell.c: shell = DEFAULT_SHELL;
--
This looks ok.
./shell/ash.c- continue;
./shell/ash.c- ap = new = ckmalloc((ap - argv + 2) * sizeof(ap[0]));
./shell/ash.c- ap[1] = cmd;
./shell/ash.c: ap[0] = cmd = (char *)DEFAULT_SHELL;
--
Need to read shell documentation to figure out what standards say.
./runit/svlogd.c- xmove_fd(fd, 5);
./runit/svlogd.c-
./runit/svlogd.c-// getenv("SHELL")?
./runit/svlogd.c: execl(DEFAULT_SHELL, DEFAULT_SHELL_SHORT_NAME,
"-c", ld->processor, (char*) NULL);
--
Should we try to use $SHELL, then try retrieve user's shell
before falling back to DEFAULT_SHELL?
./archival/libarchive/data_extract_to_command.c-
close(p[1]);
./archival/libarchive/data_extract_to_command.c-
xdup2(p[0], STDIN_FILENO);
./archival/libarchive/data_extract_to_command.c-
signal(SIGPIPE, SIG_DFL);
./archival/libarchive/data_extract_to_command.c:
execl(DEFAULT_SHELL, DEFAULT_SHELL_SHORT_NAME, "-c",
archive_handle->tar__to_command, NULL);
./archival/libarchive/data_extract_to_command.c:
bb_perror_msg_and_die("can't execute '%s'", DEFAULT_SHELL);
--
Should we try to use $SHELL, then try retrieve user's shell
before falling back to DEFAULT_SHELL?
./networking/ifupdown.c- case -1: /* failure */
./networking/ifupdown.c- return 0;
./networking/ifupdown.c- case 0: /* child */
./networking/ifupdown.c: execle(DEFAULT_SHELL,
DEFAULT_SHELL, "-c", str, (char *) NULL, G.my_environ);
--
Should we try to use $SHELL, then try retrieve user's shell
before falling back to DEFAULT_SHELL?
./coreutils/chroot.c- argv -= 2;
./coreutils/chroot.c- argv[0] = getenv("SHELL");
./coreutils/chroot.c- if (!argv[0]) {
./coreutils/chroot.c: argv[0] = (char *) DEFAULT_SHELL;
--
Should we try to retrieve current user's shell (getpwuid(getuid())->pw_shell)
before falling back to DEFAULT_SHELL?
--
./console-tools/openvt.c- argv--;
./console-tools/openvt.c- argv[0] = getenv("SHELL");
./console-tools/openvt.c- if (!argv[0])
./console-tools/openvt.c: argv[0] = (char *)
DEFAULT_SHELL;
--
Should we try to retrieve current user's shell (getpwuid(getuid())->pw_shell)
before falling back to DEFAULT_SHELL?
In short, seems like we can create a helper function and use it in most
of these locations.
Care to prepare a patch?
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
