Bug#401180: [Buildd-tools-devel] Bug#401180: schroot: using a symlink for /var/lib/schroot/mount causes /home and /tmp to be erased

[Roger Leigh]

Andreas Barth <aba at not.so.argh.org> writes:

> severity 401180 serious
> thanks
>
> * Steve Langasek (vorlon at debian.org) [061202 02:08]:
>> On Fri, Dec 01, 2006 at 02:29:29PM +0100, Lucas Nussbaum wrote:
>> Symlinking system directories that are under the control of packages is
>> always a risky affair.  I'm sorry if this resulted in real data loss for
>> you, but I don't think it's appropriate to hold schroot responsible at an
>> RC-level for all the ways that things can go wrong if you put symlinks where
>> directories are expected.

I'd also like to apologise for the data loss you suffered.  I didn't
intend for schroot to be used in this way, and this is the result of a
design assumption breaking in a nasty way.

> I think schroot should really be fixed here, and should be fixed prior
> to a stable release. I can imagine lots of people to run into the same
> situation.

The crux of the problem, as I see it, is in schroot-listmounts-main.cc
(main::list_mounts()):

  std::string to_find = sbuild::normalname(this->opts->mountpoint);

  std::FILE *mntdb = std::fopen(mountfile.c_str(), "r");
  if (mntdb == 0)
    throw error(mountfile, OPEN, strerror(errno));

  mntent *mount;
  while ((mount = getmntent(mntdb)) != 0)
    {
      std::string mount_dir(mount->mnt_dir);
      if (to_find == "/" ||
	  (mount_dir.find(to_find) == 0 &&
	   (// Names are the same.
	    mount_dir.size() == to_find.size() ||
	    // Must have a following /, or not the same directory.
	    (mount_dir.size() > to_find.size() &&
	     mount_dir[to_find.size()] == '/'))))
	ret.push_back(mount_dir);
    }

Here, we are parsing /proc/mounts using the glibc functions to walk
through all the mounted filesystems, and then unmount all the ones we
find which are under /var/lib/schroot/mounts/$session_id.  Finally, we
"rm -rf" the mountpoint.

Due to the potential for dataloss, schroot-listmounts was written to
ensure that the cleanup would fail if any unmounted filesystems were
not unmounted.  However, this cannot cope if you introduce symlinks
into the schroot mount directory path, because this is not recorded in
/proc/mounts or /etc/mtab, and it will always appear as if there are
no filesystems mounted.

One possible solution would be to use realpath(3) to canonicalise the
mount path in schroot-listmounts.  This should be quite easy to
implement.  to_find can simply be canonicalised; it's a two line fix.

However, I'm wondering if it would be better to canonicalise all paths
at all times in the main code, to prevent anything like this
reoccuring.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux             http://people.debian.org/~rleigh/
 `. `'   Printing on GNU/Linux?       http://gutenprint.sourceforge.net/
   `-    GPG Public Key: 0x25BFB848   Please GPG sign your mail.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/buildd-tools-devel/attachments/20061202/f83438dd/attachment.pgp