[SRU] Fails to generate valid image if /tmp is mounted nosuid,nodev & -t is not specified

Bug #228744 reported by Emmet Hikory
Affects Status Importance Assigned to Milestone
ubuntu-vm-builder (Ubuntu)
Fix Released
Fix Released

Bug Description

Binary package hint: ubuntu-vm-builder

ubuntu-vm-builder assumes that $TMPDIR allows suid and device files. For the case where a user has mounted both /home and /tmp with -o nodev,nosuid, this is potentially problematic. While there are a number of ways to test for this, and verify that the executing user has the necessary permissions to create the files required by the image, an easy workaround would be to mount a tmpfs at $WORKINGDIR to ensure that the target temporary location supports the needed functionality.

Revision history for this message
Emmet Hikory (persia) wrote :

Attached is a quick hack patch with the described solution. It works for me, although it might benefit from a command-line switch to disable it, and some error checking (although ubuntu-vm-builder fails back to the unpatched behaviour, with a bit of extra output, if the mount fails, so adding error checking doesn't make it more robust (unless someone sets -e or the like)).

Revision history for this message
Emmet Hikory (persia) wrote :

Err. That patch is against Nick Barcet's PPA build. Sorry for any confusion.

Revision history for this message
Nick Barcet (nijaba) wrote : Re: [Bug 228744] Re: Fails to generate valid image if /tmp is mounted nosuid, nodev & -t is not specified

Nice suggestion, thanks, will definitely apply it :)

  status triaged
  importance low

Changed in ubuntu-vm-builder:
importance: Undecided → Low
status: New → Triaged
Revision history for this message
Nick Barcet (nijaba) wrote : Re: Fails to generate valid image if /tmp is mounted nosuid,nodev & -t is not specified

Applied the fix to trunk.

Changed in ubuntu-vm-builder:
importance: Low → Undecided
status: Triaged → Fix Committed
importance: Undecided → Medium
Revision history for this message
Nick Barcet (nijaba) wrote :
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-vm-builder - 0.6

ubuntu-vm-builder (0.6) intrepid; urgency=low

  * Release into Ubuntu proper.

ubuntu-vm-builder (0.5ubuntu1~ppa4) intrepid; urgency=low

  [Loic Minier]
  * Call sh -c "$EXEC_SCRIPT" instead of "$EXEC_SCRIPT"; allows to pass
    arguments to the script; also remove check that EXEC_SCRIPT exists.

  [Nick Barcet]
  * Adding an error handler to fix LP: #217950
  * Lots of sanitization to allow for error handler
  * Add an interrupt handler to cleanup if user interrupts script
  * Stop on error in user script to fix LP: #228675
  * --ssh-key adds key to root and --ssh-user-key adds key to user
  * Added --raw option to install on raw devices/files.
    WARNING: the variables used in template files for disk definition have been
    modified. Please insure that locally created templates are updated to
    reflect this change.
  * Add --firstboot and --firstlogin options
  * First login always execute "sudo dpkg-reconfigure console-setup" so
    that the local keyboard setting is taken into account.
  * Adding the --iso parameter to create image from an iso. This requires
    suite and kernel-flavour parameters to match what is available on the iso,
  * Include hostname in default destination directory if defined
  * Do not use a tmpfs by default anymore
  * Place the working directory in the same directory as dest if using --tmp -
  * Added --tmpfs option to specify usage of a tmpfs for the working directory
  * VM specific parameters do not need to be the last ones anymore
  * Unknown parameters now return an error and prints usage
  * Added --overwrite for overwriting of destination directory and libvirt
  * Added ~/.ubuntu-vm-builder config handling
  * Man page improvements and reorganization

  [Soren Hansen]
  * Fix for LP: #234062 ssh root login broken

ubuntu-vm-builder (0.4ubuntu2~ppa7) hardy; urgency=low

  [ Michael Vogt ]
  * patch the way do_avoid_starting_daemons() to write a policy-rc.d file in
    the same way as pbuilder does (LP: #228372)

  [ Nick Barcet ]
  * Lock the root account by default (LP: #230291)
  * Add ssh keys to the user account and not to root (LP: #230291)
  * Added function do_copy_settings to fix bug LP: #221231
  * Fix missing ipv6 entries in host file (LP: #230299)
  * Fix issue with template arguments fetching (LP: #228268)
  * Create the /etc/apt/sources.list properly (LP: #218195)
  * Use a tmpfs for $WORKINGDIR to avoid case when file system is mounted
    with no suid (LP: #228744)
  * Unproper letters variable initialization (LP: #230312)
  * Option --net failed other than for Class C (LP: #232361)

  [ Loic Minier ]
  * Fix v / --verbose getopt parsing. (LP: #230319)
  * Compute the default ARCH with dpkg --print-architecture. (LP: #230323)
  * Add support for lpia.
    - Allow lpia arch, lpia and lpiacompat kernel flavours.
    - Use http://ports.ubuntu.com/ubuntu-ports as default mirror for lpia.
    - Update help/documentation.
  * Check Release files against the archive keyring; depend on ubuntu-keyring.
    (LP: #230334)

 -- Soren Hansen <email address hidden> Wed, 28 May 2008 11:36:02 +0200

Changed in ubuntu-vm-builder:
status: Fix Committed → Fix Released
Revision history for this message
Martin Pitt (pitti) wrote :

Accepted into -proposed, please test and give feedback here

Changed in ubuntu-vm-builder:
status: New → Fix Committed
Revision history for this message
Emmet Hikory (persia) wrote :

The candidate works for me. Not as well as 0.4ubuntu2~ppa7, but it can be made to work without modification of supplied code.

Revision history for this message
Soren Hansen (soren) wrote :

This fails horribly on machines with less RAM than double the size of the complete target filesystem, doesn't it?

Revision history for this message
Loïc Minier (lool) wrote :

Yes, I complained to this effect to Nick explaining that despite my laptop having 3G of RAM, I couldn't create an UME VM because it was larger than 1.5 G -- this despite having the disk space.

I first guessed tmpfs had perf improvements, but Nick pointed me at this bug with the nodev, nosuid /tmp issue.

One option would be to claim that one of /tmp or the target fs need to be mounted without these flags and report a failure otherwise.

Another option might be to loop mount a file, but it requires guessing the filesystem size; you could use a sparse 8 GB ext3 and copy over to the target fs, but that wouldn't be elegant, or you could try to loop mount the target fs.

In all cases I think tmpfs is an interesting /option/ and proposed Nick to simply not make it the default.

Revision history for this message
Soren Hansen (soren) wrote :

Well, guessing the filesystem size is not a problem. In fact, u-v-b *defines* the filesystem size in the end anyway. We just do all the work in a separate directory first and the move it into the loop-mounted filesystem images so that temporary files won't use up file system blocks and bloat the resulting filesystem images. If we stopped doing that, this would all be solved. I'll see if I can come up with a patch. The current approach is certainly wrong.

Revision history for this message
Martin Pitt (pitti) wrote :

Fixed package accepted into hardy-proposed, please test.

Revision history for this message
Steve Beattie (sbeattie) wrote :

I can confirm that the version of ubuntu-vm-builder in hardy-updates, 0.4-0ubuntu0.1, doesn't handle the situation where /tmp is mounted nodev,nosuid, and that the version in hardy-proposed, 0.4-0ubuntu0.4, with the new "--in-place" option is able to build a vm with /tmp so mounted.

(As an aside, TMPDIR as set in the host is passed along to the guest during the build, so if the guest's filesystem doesn't have the path that TMPDIR is set to in the host (e.g. "/data" or "$HOME/tmp/") then bits of the installation fail miserably. I know ubuntu-vm-builder has been replaced with a python version, but it might be worth checking if this issue still lurks there.)

tags: added: verification-done
removed: verification-needed
Revision history for this message
Martin Pitt (pitti) wrote :

Copied to hardy-updates.

Changed in ubuntu-vm-builder (Ubuntu Hardy):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers