[libvirt] [PATCH v3 08/12] LXC: controller: change the owner of tty devices to the root user of container

[Richard RW. Weinberger]

----- Ursprüngliche Mail -----
> On 05/23/2013 01:52 PM, Richard RW. Weinberger wrote:
> > Hi!
> > 
> > ----- Ursprüngliche Mail -----
> >> Since these tty devices will be used by container,
> >> the owner of them should be the root user of container.
> >>
> >> Signed-off-by: Gao feng <gaofeng at cn.fujitsu.com>
> >> ---
> >>  src/lxc/lxc_controller.c | 43
> >>  +++++++++++++++++++++++++++++++++++++------
> >>  1 file changed, 37 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> >> index 7d10660..4660f25 100644
> >> --- a/src/lxc/lxc_controller.c
> >> +++ b/src/lxc/lxc_controller.c
> >> @@ -1380,13 +1380,14 @@ static int
> >> lxcSetPersonality(virDomainDefPtr
> >> def)
> >>   * *TTYNAME.  Heavily borrowed from glibc, but doesn't require
> >>   that
> >>   * devpts == "/dev/pts" */
> >>  static int
> >> -lxcCreateTty(char *ptmx, int *ttymaster, char **ttyName)
> >> +lxcCreateTty(virLXCControllerPtr ctrl, int *ttymaster,
> >> +             char **ttyName, char **ttyHostPath)
> >>  {
> >>      int ret = -1;
> >>      int ptyno;
> >>      int unlock = 0;
> >>  
> >> -    if ((*ttymaster = open(ptmx, O_RDWR|O_NOCTTY|O_NONBLOCK)) <
> >> 0)
> >> +    if ((*ttymaster = open(ctrl->devptmx,
> >> O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0)
> >>          goto cleanup;
> >>  
> >>      if (ioctl(*ttymaster, TIOCSPTLCK, &unlock) < 0)
> >> @@ -1407,6 +1408,13 @@ lxcCreateTty(char *ptmx, int *ttymaster,
> >> char
> >> **ttyName)
> >>          goto cleanup;
> >>      }
> >>  
> >> +    if (virAsprintf(ttyHostPath, "/%s/%s.devpts/%d",
> >> LXC_STATE_DIR,
> >> +                    ctrl->def->name, ptyno) < 0) {
> >> +        virReportOOMError();
> >> +        errno = ENOMEM;
> >> +        goto cleanup;
> >> +    }
> >> +
> >>      ret = 0;
> >>  
> >>  cleanup:
> >> @@ -1552,18 +1560,41 @@
> >> virLXCControllerSetupConsoles(virLXCControllerPtr ctrl,
> >>                                char **containerTTYPaths)
> >>  {
> >>      size_t i;
> >> +    int ret = -1;
> >> +    uid_t uid = (uid_t)-1;
> >> +    gid_t gid = (gid_t)-1;
> >> +    char *ttyHostPath = NULL;
> >> +
> >> +    if (ctrl->def->idmap.uidmap) {
> >> +        uid = ctrl->def->idmap.uidmap[0].target;
> >> +        gid = ctrl->def->idmap.gidmap[0].target;
> >> +    }
> >>  
> >>      for (i = 0; i < ctrl->nconsoles; i++) {
> >>          VIR_DEBUG("Opening tty on private %s", ctrl->devptmx);
> >> -        if (lxcCreateTty(ctrl->devptmx,
> >> +        if (lxcCreateTty(ctrl,
> >>                           &ctrl->consoles[i].contFd,
> >> -                         &containerTTYPaths[i]) < 0) {
> >> +                         &containerTTYPaths[i], &ttyHostPath) <
> >> 0) {
> >>              virReportSystemError(errno, "%s",
> >>                                       _("Failed to allocate
> >>                                       tty"));
> >> -            return -1;
> >> +            goto out;
> >>          }
> >> +
> >> +        /* Change the owner of tty device to the root user of
> >> container */
> >> +        if (chown(ttyHostPath, uid, gid) < 0) {
> >> +            virReportSystemError(errno,
> >> +                                 _("Failed to change owner of
> >> tty"
> >> +                                 " %s to %u:%u"),
> >> +                                 ttyHostPath, uid, gid);
> >> +            goto out;
> >> +        }
> >> +        VIR_FREE(ttyHostPath);
> > 
> > Why do you free ttyHostPath here?
> > You already do it in the exit path.
> > 
> 
> It has a cycle here,
> we need free the ttyHostPath since we allocate it in lxcCreateTty
> every cycle.
> 
> >>      }
> >> -    return 0;
> >> +
> >> +    ret = 0;
> >> +out:
> >> +    VIR_FREE(ttyHostPath);
> > 
> > Double free?
> > 
> 
> Don't worry about it, VIR_FREE does some extra jobs for us. ;)

Ahhh, there is some hidden magic. Now it makes sense. :D

Thanks,
//richard