bug#18736: chroot regression - chroot avoids the chroot() call too eagerly.

[Pádraig Brady]

On 10/15/2014 10:55 PM, Bernhard Voelker wrote:
> On 10/15/2014 07:17 PM, Pádraig Brady wrote:
>> I agree with your analysis and that we should revert
>> to the previous behavior here, which is done in
>> the attached patch.
> 
> Hi Padraig,
> 
> I also agree that chroot(1) should chroot(2) in such a case, but wouldn't
> be the obvious fix to STREQ() the canonicalized DIR against "/" rather
> than reverting the whole change - something like the following?
> 
> Have a nice day,
> Berny
> 
> diff --git a/src/chroot.c b/src/chroot.c
> index 171ced9..7f60106 100644
> --- a/src/chroot.c
> +++ b/src/chroot.c
> @@ -175,7 +175,13 @@ is_root (const char* dir)
>      error (EXIT_CANCELED, errno, _("failed to get attributes of %s"),
>             quote (dir));
> 
> -  return SAME_INODE (root_ino, arg_st);
> +  if (! SAME_INODE (root_ino, arg_st))
> +    return false;
> +
> +  char *resolved = canonicalize_file_name (dir);
> +  bool is_res_root = resolved && STREQ ("/", resolved);
> +  free (resolved);
> +  return is_res_root;
>  }

Yes I considered that and it should work for this case.
BTW the inode check would then be moot right?

Doing that would give better performance for --userspec=... --skip-chdir
and provide consistency for non root `chroot /` across platforms.
I'm worried though there are other edge cases we've not considered,
and the benefits above are not worth the risk?

Pádraig.