Re: [PATCH 3/3] trans: remove the loop detection logic from the mtab translator

[Samuel Thibault]

Justus Winter, le Wed 20 Nov 2013 11:10:08 +0100, a écrit :
> Previously the mtab detector tried to detect whether it talks to
> itself by comparing the control port of the translator it currently
> visits with its own. This was done to prevent deadlocks. However, this
> did not detect non-trivial loops, i.e. two mtab translators would
> still dead-lock each other.
> 
> Now that it is safe to actually file_name_lookup the root node of a
> mtab translator without triggering the generation of content, this
> logic can just be omitted.

Ack.

> * trans/mtab.c (mtab_populate): Remove the loop detection logic.
> ---
>  trans/mtab.c |   43 +++++++++++--------------------------------
>  1 file changed, 11 insertions(+), 32 deletions(-)
> 
> diff --git a/trans/mtab.c b/trans/mtab.c
> index b1a3939..d3c3583 100644
> --- a/trans/mtab.c
> +++ b/trans/mtab.c
> @@ -286,7 +286,6 @@ mtab_populate (struct mtab *mtab, const char *path, int 
> insecure)
>  
>    /* These resources are freed in the epilogue.       */
>    file_t node = MACH_PORT_NULL;
> -  fsys_t fsys = MACH_PORT_NULL;
>    char *argz = NULL;
>    size_t argz_len = 0;
>    char **argv = NULL;
> @@ -299,16 +298,16 @@ mtab_populate (struct mtab *mtab, const char *path, int 
> insecure)
>    char *children = NULL;
>    size_t children_len = 0;
>  
> -  /* Get the underlying node.  */
> -  node = file_name_lookup (path, O_NOTRANS, 0666);
> -  if (node == MACH_PORT_NULL)
> -    {
> -      err = errno;
> -      goto errout;
> -    }
> -
>    if (! insecure)
>      {
> +      /* Get the underlying node.  */
> +      node = file_name_lookup (path, O_NOTRANS, 0666);
> +      if (node == MACH_PORT_NULL)
> +        {
> +          err = errno;
> +          goto errout;
> +        }
> +
>        /* Check who owns the node the translator is bound to.  */
>        io_statbuf_t st;
>        err = io_stat (node, &st);
> @@ -320,28 +319,11 @@ mtab_populate (struct mtab *mtab, const char *path, int 
> insecure)
>         err = EPERM;
>         goto errout;
>       }
> -    }
>  
> -  err = file_get_translator_cntl (node, &fsys);
> -  if (err == EPERM)
> -    /* If we do not have permission to do that, it cannot be a node
> -       bound to our control port, so ignore this error.       */
> -    err = 0;
> -
> -  if (err == ENXIO && strcmp (path, "/") == 0)
> -    /* The root translator fails differently, but this can't be bound
> -       to our control port either, so ignore this error.  */
> -    err = 0;
> -
> -  if (err)
> -    return err;
> -
> -  if (control && control->pi.port_right == fsys)
> -    /* This node is bound to our control port, ignore it.  */
> -    goto errout;
> +      mach_port_deallocate (mach_task_self (), node);
> +    }
>  
> -  /* Re-do the lookup without O_NOTRANS to get the root node.  */
> -  mach_port_deallocate (mach_task_self (), node);
> +  /* (Re-)do the lookup without O_NOTRANS to get the root node.  */
>    node = file_name_lookup (path, 0, 0666);
>    if (node == MACH_PORT_NULL)
>      {
> @@ -465,9 +447,6 @@ mtab_populate (struct mtab *mtab, const char *path, int 
> insecure)
>    if (node != MACH_PORT_NULL)
>      mach_port_deallocate (mach_task_self (), node);
>  
> -  if (fsys != MACH_PORT_NULL)
> -    mach_port_deallocate (mach_task_self (), fsys);
> -
>    if (argz)
>      vm_deallocate (mach_task_self (), (vm_address_t) argz, argz_len);
>  
> -- 
> 1.7.10.4
> 

-- 
Samuel
<A>  mr  -  remove the home of correct users who accidentally enter mr
<A>        instead of rm