Bug#369626: [Buildd-tools-devel] Bug#369626: schroot: rm -rf in file
chroot cleanup destroys real /home if umount fails
Roger Leigh
rleigh at whinlatter.ukfsn.org
Fri Jun 9 17:40:49 UTC 2006
Andreas Bombe <bombe at informatik.tu-muenchen.de> writes:
> On Wed, May 31, 2006 at 10:53:18AM +0100, Roger Leigh wrote:
>> Andreas Bombe <aeb at debian.org> writes:
>>
>> > The session cleanup in 10mount ignores failures of umount invocations
>> > and cleanup continues. In the case of file chroots with a /home bind
>> > mount that failed to umount, the rm -rf in 05file blindly descends into
>> > the system /home with obvious unpretty results.
>> There are a few possibilities here.
>>
>> 1) 10mount should exit with an error if umount fails.
>>
>> Caveat: if the session is ended with the setup scripts having
>> failed, this would require manual cleanup by the system admin.
>> This needs additional work in session::setup_chroot() in
>> sbuild-session.cc, so that the session is not ended if the scripts
>> fail. This means not removing the session file from
>> /var/lib/schroot/session/ on failure.
>>
>> Currently, because of the above consideration, the "setup-stop"
>> phase of the session scripts can not fail.
>
> If a umount fails it will require manual admin intervention anyway so
> that wouldn't make much of a difference. Making the rm -rf safe is
> still required in any case, I'd say.
OK, I agree here. In the patch below, any umount failure will abort
the cleanup.
>> 2) 05file must check if any filesystems are mounted under the chroot
>> root before running rm -rf. Is there a portable and reliable way
>> of doing this? Would
>>
>> if mount | grep "$CHROOT_MOUNT_LOCATION"; then
>> :
>> else
>> rm -rf "$CHROOT_MOUNT_LOCATION" || true
>> fi
>>
>> be sufficient?
>
> I don't think that is safe. It depends on all mounts being recorded in
> /etc/mtab, which is not the case if something *inside* the chroot
> mounted something, for example.
> I thought about rm with a "do not cross filesystems" option, still that
> wouldn't help because binds may well be on the same filesystem. There
> are no usable criteria for using "find ... -exec rm ..." either.
To work around this, I wrote a program called schroot-listmounts,
which uses /proc/mounts to get a list:
./schroot/schroot-listmounts -m /srv/chroot/sid
/srv/chroot/sid/tmp
/srv/chroot/sid/home
/srv/chroot/sid/dev/shm
/srv/chroot/sid/dev/pts
/srv/chroot/sid
The shell script can read this list and unmount them in turn.
> The only way I know to be sure there are no submounts is to mount --bind
> the chroot to a temporary location and rm -rf there, then unmount the
> temporary bind and rmdir the chroot.
>
> The rmdir will fail safely if the chroot isn't empty then. Even before,
> the rm -rf of the temp bind will fail safely upon trying to remove an
> empty directory used as a mount point in the chroot.
I think I now have this solved. If the chroot directories are
unmounted, schroot-listmounts will list nothing:
./schroot/schroot-listmounts -m /var/lib/schroot/mount/testsnap-c4e53f96-e507-40e8-8cf4-fcfd31c17c32
[no output]
so in this case we can be sure it's safe to "rm -rf". If there's any
output at all, we know it's not safe.
I've attached a patch for this. Note: it's not yet tested; this is
just to give you an idea of what I'm thinking of as a solution. If
the testing works out, and you are happy with it, I'll include this in
the next release.
ChangeLog | 43 ++++++++
NEWS | 10 +
debian/changelog | 3
schroot/Makefile.am | 10 +
schroot/sbuild-chroot-block-device.cc | 3
schroot/sbuild-chroot-block-device.h | 9 -
schroot/sbuild-chroot-file.cc | 5
schroot/sbuild-chroot-file.h | 9 -
schroot/sbuild-chroot-lvm-snapshot.cc | 5
schroot/sbuild-chroot-lvm-snapshot.h | 9 -
schroot/sbuild-chroot-plain.cc | 5
schroot/sbuild-chroot-plain.h | 9 -
schroot/sbuild-chroot.cc | 13 ++
schroot/sbuild-chroot.h | 42 +++++++
schroot/sbuild-session.cc | 6 -
schroot/sbuild-util.cc | 13 ++
schroot/sbuild-util.h | 12 ++
schroot/schroot-listmounts-options.cc | 83 +++++++++++++++
schroot/schroot-listmounts-options.h | 59 +++++++++++
schroot/schroot-listmounts.cc | 181 ++++++++++++++++++++++++++++++++++
schroot/setup/05file | 15 ++
schroot/setup/10mount | 31 +----
test/sbuild-chroot.cc | 3
23 files changed, 522 insertions(+), 56 deletions(-)
Regards,
Roger
--
Roger Leigh
Printing on GNU/Linux? http://gutenprint.sourceforge.net/
Debian GNU/Linux http://www.debian.org/
GPG Public Key: 0x25BFB848. Please sign and encrypt your mail.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: schroot-safecleanup.patch
Type: text/x-patch
Size: 25825 bytes
Desc: Safe unmount and cleanup patch
Url : http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20060609/1568e651/schroot-safecleanup-0001.bin
More information about the Buildd-tools-devel
mailing list