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

Previous Next

Package: coreutils;

Reported by: Rogier <rogier777 <at> gmail.com>

Date: Wed, 15 Oct 2014 15:44:04 UTC

Severity: normal

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

Full log


Message #23 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 11:02:07 +0100
On 10/16/2014 08:38 AM, Bernhard Voelker wrote:
> Hi Padraig,
> 
> On 10/16/2014 02:05 AM, Pádraig Brady wrote:
>>  From d520929586ee2063d48359aaaef8f28807604cae Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?P=C3=A1draig=20Brady?=<P <at> draigBrady.com>
>> Date: Wed, 15 Oct 2014 18:08:42 +0100
>> Subject: [PATCH] chroot: call chroot() unconditionally to handle bind mounted
>>   "/"
>>
>> * src/chroot.c (is_root): Adjust to compare canonicalized paths
>> rather than inodes, to handle (return false in) the case where
>> we have a tree that is constructed by first bind mounting "/"
>> (thus having the same inode).
>> (main): Unconditionally call chroot() because it's safer
>> and of minimal performance benefit to avoid in this case.
>> This will cause inconsistency with some platforms
>> not allowing `chroot / true` for non root users.
> 
> I'm not sure that introducing the inconsistency again is the right
> way to go, although I don't have a strong preference (40:60):
> I'd probably go with the smaller change in is_root() and wait for
> other edge cases to come (which I think is improbable).

I don't have a strong preference either but would be (60:40) the other way.
In my mind they comes under the "it's better to ask forgiveness than permission" idea.
I.E. it's better to invoke lower layer logic where possible, rather than
adding higher level logic. Given it's more risky to avoid the chroot()
I'd be inclined to wait until there were complaints about the inconsistent
behavior rather than the other way around.


>> diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh

> Minor nit: it's better to initialize 'can_chroot_root' here.

Good point. Done.

thanks,
Pádraig.





This bug report was last modified 10 years and 277 days ago.

Previous Next


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