GNU bug report logs - #18736
chroot regression - chroot avoids the chroot() call too eagerly.

Previous Next

Full log

Message #17 received at 18736 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>
Cc: 18736 <at> debbugs.gnu.org, Rogier <rogier777 <at> gmail.com>
Subject: Re: bug#18736: chroot regression - chroot avoids the chroot() call
 too eagerly.
Date: Thu, 16 Oct 2014 01:05:21 +0100

[Message part 1 (text/plain, inline)]

On 10/16/2014 12:14 AM, Pádraig Brady wrote:
> 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?

I suppose a compromise is to keep most of the better performance for:

  chroot --userspec=... --skip-chdir /

but do the chroot(2) unconditionally.

Updated patch is attached.

thanks,
Pádraig.

[chroot-always.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.