[Devel] [PATCH] vzctl: simplification stdout/err redirection for vzctl exec
Kir Kolyshkin
kir at parallels.com
Mon Oct 6 03:21:28 PDT 2014
Kostya, all,
vzctl exec is implemented this way (two pairs of terminals with a proxy
in between)
because otherwise a container can access the host terminal, which is a
security violation.
In short, it will make it possible to run commands from host console (as
root).
If you need a proper terminal, use vzctl enter rather than exec. Yet better,
use ssh as a standard way to access your container.
Kir.
On 10/01/2014 02:41 AM, Konstantin Neumoin wrote:
> From: Konstantin Neumoin <kneumoin at gmail.com>
>
> good point: vzctl need just do fork and wait child exit or for a timeout
> negative: bash "cannot set terminal process group"
> example:
>
>> vzctl exec 101 bash
>> bash: cannot set terminal process group (-1): Inappropriate ioctl for device
>> bash: no job control in this shell
>> bash-4.1# echo 1
>> 1
>> bash-4.1# exit
>> exit
>> vzctl exec 101 bash
>> echo 1
>> 1
>> exit
> Signed-off-by: Konstantin Neumoin <kneumoin at parallels.com>
> ---
> src/lib/exec.c | 215 ++++++++++++---------------------------------------------
> 1 file changed, 46 insertions(+), 169 deletions(-)
>
> diff --git a/src/lib/exec.c b/src/lib/exec.c
> index 84648a1..b2f7d40 100644
> --- a/src/lib/exec.c
> +++ b/src/lib/exec.c
> @@ -77,49 +77,6 @@ int execvep(const char *path, char *const argv[], char *const envp[])
> return -1;
> }
>
> -static int stdredir(int rdfd, int wrfd)
> -{
> - int lenr, lenw, lentotal, lenremain;
> - char buf[10240];
> - char *p;
> -
> - if ((lenr = read(rdfd, buf, sizeof(buf)-1)) > 0) {
> - lentotal = 0;
> - lenremain = lenr;
> - p = buf;
> - while (lentotal < lenr) {
> - if ((lenw = write(wrfd, p, lenremain)) < 0) {
> - if (errno != EINTR)
> - return -1;
> - } else {
> - lentotal += lenw;
> - lenremain -= lenw;
> - p += lenw;
> - }
> - }
> - } else if (lenr == 0){
> - return -1;
> - } else {
> - if (errno == EAGAIN)
> - return 1;
> - else if (errno != EINTR)
> - return -1;
> - }
> - return 0;
> -}
> -
> -static void exec_handler(int sig)
> -{
> - child_exited = 1;
> - return;
> -}
> -
> -static void alarm_handler(int sig)
> -{
> - alarm_flag = 1;
> - return;
> -}
> -
> int env_wait(int pid)
> {
> int ret, status;
> @@ -152,63 +109,44 @@ static int vps_real_exec(vps_handler *h, envid_t veid, const char *root,
> int timeout)
> {
> int ret, pid;
> - int in[2], out[2], err[2], st[2];
> - int fl = 0;
> struct sigaction act;
> char *def_argv[] = { NULL, NULL };
> + sigset_t mask, oldmask;
> + struct timespec sigtime = { timeout, NULL };
>
> - if (pipe(in) < 0 ||
> - pipe(out) < 0 ||
> - pipe(err) < 0 ||
> - pipe(st) < 0)
> - {
> - logger(-1, errno, "Unable to create pipe");
> - return VZ_RESOURCE_ERROR;
> - }
> /* Default for envp if not set */
> - if (envp == NULL)
> + if (envp == NULL) {
> envp = envp_bash;
> - /* Set non block mode */
> - set_not_blk(out[0]);
> - set_not_blk(err[0]);
> - /* Setup alarm handler */
> - if (timeout) {
> - alarm_flag = 0;
> - act.sa_flags = 0;
> - sigemptyset(&act.sa_mask);
> - act.sa_handler = alarm_handler;
> - sigaction(SIGALRM, &act, NULL);
> - alarm(timeout);
> }
> - child_exited = 0;
> - sigemptyset(&act.sa_mask);
> - act.sa_flags = SA_NOCLDSTOP;
> - act.sa_handler = exec_handler;
> - sigaction(SIGCHLD, &act, NULL);
>
> - act.sa_handler = SIG_IGN;
> - act.sa_flags = 0;
> - sigaction(SIGPIPE, &act, NULL);
> + sigemptyset(&mask);
> + sigaddset(&mask, SIGCHLD);
> + if(sigprocmask(SIG_BLOCK, &mask, &oldmask)) {
> + logger(-1, errno, "Unable set mask");
> + return VZ_RESOURCE_ERROR;
> + }
> + sigaddset(&mask, SIGPIPE);
>
> if ((ret = h->setcontext(veid)))
> return ret;
> +
> + fflush(stdout);
> if ((pid = fork()) < 0) {
> logger(-1, errno, "Unable to fork");
> - ret = VZ_RESOURCE_ERROR;
> - goto err;
> + return VZ_RESOURCE_ERROR;
> } else if (pid == 0) {
> - close(0); close(1); close(2);
> - dup2(in[0], STDIN_FILENO);
> - dup2(out[1], STDOUT_FILENO);
> - dup2(err[1], STDERR_FILENO);
> -
> - close(in[0]); close(out[1]); close(err[1]);
> - close(in[1]); close(out[0]); close(err[0]);
> - close(st[0]);
> - fcntl(st[1], F_SETFD, FD_CLOEXEC);
> - close_fds(0, st[1], h->vzfd, -1);
> + sigprocmask(SIG_SETMASK, &oldmask, NULL);
> + if (std_in != NULL) {
> + int in[2];
> + if (pipe(in) < 0)
> + exit(VZ_RESOURCE_ERROR);
> + dup2(in[0], STDIN_FILENO);
> + if (write(in[1], std_in, strlen(std_in)) < 0)
> + exit(VZ_COMMAND_EXECUTION_ERROR);
> + }
> + close_fds(0, h->vzfd, -1);
> if ((ret = h->enter(h, veid, root, 0)))
> - goto env_err;
> + exit(ret);
> if (exec_mode == MODE_EXEC && argv != NULL) {
> execvep(argv[0], argv, envp);
> } else {
> @@ -219,93 +157,32 @@ static int vps_real_exec(vps_handler *h, envid_t veid, const char *root,
> argv[0] = "/bin/sh";
> execve(argv[0], argv, envp);
> }
> - ret = VZ_FS_BAD_TMPL;
> -env_err:
> - write(st[1], &ret, sizeof(ret));
> - exit(ret);
> + exit(VZ_FS_BAD_TMPL);
> }
> - close(st[1]);
> - close(out[1]); close(err[1]); close(in[0]);
> - while ((ret = read(st[0], &ret, sizeof(ret))) == -1)
> - if (errno != EINTR)
> - break;
> - if (ret)
> - goto err;
> - if (std_in != NULL) {
> - if (write(in[1], std_in, strlen(std_in)) < 0) {
> - ret = VZ_COMMAND_EXECUTION_ERROR;
> - /* Flush fd */
> - while (stdredir(out[0], STDOUT_FILENO) == 0);
> - while (stdredir(err[0], STDERR_FILENO) == 0);
> - goto err;
> - }
> - close(in[1]);
> - /* do not set STDIN_FILENO in select() */
> - fl = 4;
> - }
> - while(!child_exited) {
> - int n;
> - fd_set rd_set;
> -
> - if (timeout && alarm_flag) {
> - logger(-1, 0, "Execution timeout expired");
> - kill(pid, SIGTERM);
> - alarm(0);
> - break;
> - }
> - if ((fl & 3) == 3) {
> - /* all fd are closed */
> - close(in[1]);
> - break;
> - }
> - FD_ZERO(&rd_set);
> - if (!(fl & 4))
> - FD_SET(STDIN_FILENO, &rd_set);
> - if (!(fl & 1))
> - FD_SET(out[0], &rd_set);
> - if (!(fl & 2))
> - FD_SET(err[0], &rd_set);
> - n = select(FD_SETSIZE, &rd_set, NULL, NULL, NULL);
> - if (n > 0) {
> - if (FD_ISSET(out[0], &rd_set))
> - if (stdredir(out[0], STDOUT_FILENO) < 0) {
> - fl |= 1;
> - close(out[0]);
> - }
> - if (FD_ISSET(err[0], &rd_set))
> - if (stdredir(err[0], STDERR_FILENO) < 0) {
> - fl |= 2;
> - close(err[0]);
> - }
> - if (FD_ISSET(STDIN_FILENO, &rd_set))
> - if (stdredir(STDIN_FILENO, in[1]) < 0) {
> - fl |= 4;
> - close(in[1]);
> - }
> - } else if (n < 0 && errno != EINTR) {
> - logger(-1, errno, "Error in select()");
> - close(out[0]);
> - close(err[0]);
> - break;
> + do {
> + if (timeout)
> + ret = sigtimedwait(&mask, NULL, &sigtime);
> + else
> + ret = sigwaitinfo(&mask, NULL);
> + if (ret == -1) {
> + if (errno == EINTR) {
> + logger(-1, 0, "Unexpected signal %d\n", ret);
> + continue;
> + }
> + else if (errno == EAGAIN) {
> + logger(-1, 0, "Execution timeout expired");
> + kill(pid, SIGKILL);
> + ret = VZ_EXEC_TIMEOUT;
> + goto err;
> + }
> }
> - }
> - /* Flush fds */
> - if (!(fl & 1)) {
> - while (stdredir(out[0], STDOUT_FILENO) == 0);
> - }
> - if (!(fl & 2)) {
> - while (stdredir(err[0], STDERR_FILENO) == 0);
> - }
> + if (ret == SIGPIPE)
> + continue;
> + break;
> + } while(1);
> ret = env_wait(pid);
> - if (ret && timeout && alarm_flag)
> - ret = VZ_EXEC_TIMEOUT;
> -
> err:
> - close(st[0]); close(st[1]);
> - close(out[0]); close(out[1]);
> - close(err[0]); close(err[1]);
> - close(in[0]); close(in[1]);
> -
> + sigprocmask(SIG_SETMASK, &oldmask, NULL);
> return ret;
> }
>
More information about the Devel
mailing list