schroot session recovery mounts $device, not $mount-device, for LVM snapshots

[Steve Langasek]

When schroot --recover-session is called (e.g., on boot), instead of mounting the lvm snapshot, the snapshot source is mounted. This is not a recovery of the session in any meaningful sense.

/var/lib/schroot/session/quantal-ca32679d-0efd-4c21-b77b-7431228aad42 has all the necessary information about the snapshot, so it seems this should be an easy fix.

ProblemType: Bug
DistroRelease: Ubuntu 12.10
Package: schroot 1.6.1-1
ProcVersionSignature: Ubuntu 3.5.0-17.28-generic 3.5.5
Uname: Linux 3.5.0-17-generic x86_64
ApportVersion: 2.6.1-0ubuntu3
Architecture: amd64
Date: Mon Oct 22 13:04:59 2012
InstallationMedia: Ubuntu 10.04.1 LTS "Lucid Lynx" - Release amd64 (20100816.1)
SourcePackage: schroot
UpgradeStatus: Upgraded to quantal on 2012-06-11 (133 days ago)
modified.conffile..etc.schroot.default.fstab: [modified]
modified.conffile..etc.schroot.schroot.conf: [modified]
modified.conffile..etc.schroot.setup.d.20nssdatabases: [modified]
modified.conffile..etc.schroot.setup.d.70services: [deleted]
mtime.conffile..etc.schroot.default.fstab: 2011-08-12T15:05:25
mtime.conffile..etc.schroot.schroot.conf: 2012-07-30T23:05:05.331645
mtime.conffile..etc.schroot.setup.d.20nssdatabases: 2012-06-11T08:50:33.413444

[Roger Leigh (rleigh)]

Could you please provide the session file, from /var/lib/schroot/session? The original config from /etc/schroot would also be useful.

schroot --config -c chroot:$origchroot -c session:$session

will provide both of these. If it's not Ubuntu-specific, the Debian BTS would also be more appropriate.

Thanks,
Roger

[Steve Langasek (vorlon)]

I don't have the session file for the original one that was recovered because it's been reaped now, but here's the config for another instance of the same chroot.

[Roger Leigh (rleigh)]

Thanks.

I can reproduce this with 1.6.3. Still working out where this is happening though.

- it's set correctly in the session file (as you've shown)
- it's also set correctly on loading (again with --config/--info, so no serialisation errors)

So something subtle is happening after that, but prior to running the setup scripts. I'll investigate this more to find out where this occurs. Possibly some odd interaction between the chroot facets, or something triggering a call to sbuild::chroot_facet_mountable::set_mount_device() which is ultimately where this problem lies.

[Roger Leigh (rleigh)]

Unwanted side effect of cloning; triggered by the copy constructor chaining up to the block_device_base ctor, which calls set_device, which then calls chroot_facet_mountable::set_mount_device.

(gdb) run --recover-session -c snaptest
Starting program: /home/rleigh/schroot/bin/schroot/schroot --recover-session -c snaptest
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Breakpoint 1, sbuild::session::set_user_options (this=0x54b830,
    user_options=...) at sbuild-session.cc:330
330 this->user_options = user_options;
(gdb) break sbuild::chroot_facet_mountable::set_mount_device
Breakpoint 2 at 0x4ad960: file sbuild-chroot-facet-mountable.cc, line 74.
(gdb) c
Continuing.

Breakpoint 2, sbuild::chroot_facet_mountable::set_mount_device (this=0x595e20,
    mount_device=...) at sbuild-chroot-facet-mountable.cc:74
74 this->mount_device = mount_device;
(gdb) bt
#0 sbuild::chroot_facet_mountable::set_mount_device (this=0x595e20,
    mount_device=...) at sbuild-chroot-facet-mountable.cc:74
#1 0x00000000004a58a5 in sbuild::chroot_block_device_base::set_device (
    this=this@entry=0x595b20, device=...)
    at sbuild-chroot-block-device-base.cc:83
#2 0x00000000004a595f in sbuild::chroot_block_device_base::chroot_block_device_base (this=0x595b20, rhs=...) at sbuild-chroot-block-device-base.cc:54
#3 0x00000000004a7d35 in sbuild::chroot_lvm_snapshot::chroot_lvm_snapshot (
    this=0x595b20, rhs=...) at sbuild-chroot-lvm-snapshot.cc:50
#4 0x00000000004a7f31 in sbuild::chroot_lvm_snapshot::clone (this=0x586550)
    at sbuild-chroot-lvm-snapshot.cc:61
#5 0x00000000004634f3 in sbuild::session::run_impl (this=0x54b830)
    at sbuild-session.cc:660
#6 0x000000000045e7d4 in sbuild::session::run (this=0x54b830)
    at sbuild-session.cc:585
#7 0x000000000042b77f in schroot::main_base::run_impl (this=0x7fffffffe9e0)
    at schroot-main-base.cc:360
#8 0x000000000041988a in schroot_base::main::run (this=0x7fffffffe9e0,
    argc=<optimized out>, argv=0x7fffffffeb78) at schroot-base-main.cc:107
#9 0x0000000000414bef in run<schroot::options, schroot::main> (
    argv=0x7fffffffeb78, argc=4)
    at ../../bin/schroot-base/schroot-base-run.h:69
#10 main (argc=4, argv=0x7fffffffeb78) at schroot.cc:43

[Roger Leigh (rleigh)]

We do in fact already take steps to ensure we don't call this when using LVM snapshots to avoid the exact issue that you've reported. However, it looks like in your case this is not working. As you can see, set_mount_device should be called for all chroot types /except/ lvm-snapshot (where the dynamic_cast<> will fail)...

sbuild::chroot_block_device_base::set_device():

  /// @todo: This may not be appropriate for derived classes such as
  /// lvm_snapshot, since re-setting the device could overwrite the
  /// mount device.
  chroot_facet_mountable::ptr pmnt
    (get_facet<chroot_facet_mountable>());
#ifdef SBUILD_FEATURE_LVMSNAP
  if (!dynamic_cast<chroot_lvm_snapshot *>(this))
#endif
    pmnt->set_mount_device(this->device);

and preprocessed:

void
chroot_block_device_base::set_device (std::string const& device)
{
  if (!is_absname(device))
    throw error(device, DEVICE_ABS);

  this->device = device;

  chroot_facet_mountable::ptr pmnt
    (get_facet<chroot_facet_mountable>());

  if (!dynamic_cast<chroot_lvm_snapshot *>(this))

    pmnt->set_mount_device(this->device);
}

[Roger Leigh (rleigh)]

OK, so this check is failing. dynamic_cast<>() is returning null because this is being called from the constructor, and the vptr during this stage of construction does not contain the information about the derived class. So we end up setting the mount device inappropriately.

This was working previously. I can only assume that this is a change due to a GCC upgrade, though it appears to affect both 4.6 and 4.7.

The code is definitely not following the letter of the C++ standard, so it will require refactoring to remove this logic entirely.

[Roger Leigh (rleigh)]

This is now being tracked in Debian bug #691376.

[Roger Leigh (rleigh)]

http://anonscm.debian.org/gitweb/?p=buildd-tools/schroot.git;a=commitdiff;h=02fa2aefc24e95852aa991f720b3c9dc53583d1c

Note that this patch needs backporting to the schroot-1.6 branch and testing with all block-device-using chroot types before I would recommend applying it. **Please do not apply this until it is fully tested.**

[Roger Leigh (rleigh)]

I'm not sure this is of low severity. I've marked this as serious in the Debian bug since it could allow a user to gain unauthorised root access to the source volume (if they were a root user for the snapshot), and potentially result in dataloss if the user didn't realise they were not using the snapshot.

[Steve Langasek (vorlon)]

Well, anyone relying on root access not leaking into the parent system is fooling themselves, so I don't think there's a security bug here. The risk for self-inflicted data loss probably makes it at least 'medium' though.

[Roger Leigh (rleigh)]

You can now get 1.6.4-2 from Debian unstable or from git

http://anonscm.debian.org/gitweb/?p=buildd-tools/schroot-dist.git;a=commit;h=8088a3b7e003546e3315957f6091553f4d2136ad

http://anonscm.debian.org/gitweb/?p=buildd-tools/schroot.git;a=commit;h=5fbab5b020922a454a1a8824ee6920e0ea26e088

(release tarball is distribution/schroot-1.6.4; delta for the -2 release is debian/schroot-1.6.4-1..debian/schroot-1.6.4-2)

Regards,
Roger