Load of pre-upgrade qemu modules needs to avoid noexec

Bug #1913421 reported by Christian Ehrhardt  on 2021-01-27
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
qemu (Ubuntu)
Undecided
Unassigned
Bionic
Undecided
Unassigned
Focal
Undecided
Unassigned
Groovy
Undecided
Unassigned

Bug Description

This is a continuation of bug 1847361.

Since that is in Ubuntu and Debian we are:
- correctly saving the modules to those paths in /var/run/qemu.
- qemu tries to load from that path as fallback
- that works fine in containers running qemu/kvm

But there is an issue on non-container systems as /run usually is like this:

  tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=3274920k,mode=755)

The important bit here is the "noexec" which is intentional (for security reasons), but prevents the loading of shared objects from that path.

The path is good for many reasons (it is auto-cleaned, upstream and Distros agreed to this one path, ...). Moving it to other places also quite likely might have unpredictable options.

In a discussion between Victor (thanks for all the pushign and inpot on this) and Marc (security POV) we have come to a solution that will make just the subpath that is owned by qemu to not have noexec set.

This bug shall track preparing this fix for Debian / Ubuntu and the latter SRu considerations on the same.

Related branches

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in qemu (Ubuntu Bionic):
status: New → Confirmed
Changed in qemu (Ubuntu Focal):
status: New → Confirmed
Changed in qemu (Ubuntu Groovy):
status: New → Confirmed
Changed in qemu (Ubuntu):
status: New → Confirmed
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Changed in qemu (Ubuntu):
status: Confirmed → In Progress
tags: added: server-next
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Build in PPA complete, testing ...

$ ll /var/run/qemu/
ls: cannot access '/var/run/qemu/': No such file or directory
$ sudo apt install --reinstall qemu-block-extra
$ ls -laFd /var/run/qemu; ls -laF /var/run/qemu; ls -laF /var/run/qemu/*
drwxr-xr-x 3 root root 60 Jan 27 20:02 /var/run/qemu/
total 0
drwxr-xr-x 3 root root 60 Jan 27 20:02 ./
drwxr-xr-x 32 root root 960 Jan 27 20:02 ../
drwxr-xr-x 2 root root 120 Jan 27 20:02 Debian_1_5.2+dfsg-3ubuntu1/
total 164
drwxr-xr-x 2 root root 120 Jan 27 20:02 ./
drwxr-xr-x 3 root root 60 Jan 27 20:02 ../
-rw-r--r-- 1 root root 38632 Jan 5 11:43 block-curl.so
-rw-r--r-- 1 root root 45160 Jan 5 11:43 block-iscsi.so
-rw-r--r-- 1 root root 35912 Jan 5 11:43 block-rbd.so
-rw-r--r-- 1 root root 40136 Jan 5 11:43 block-ssh.so

But noexec:
mount | grep run
tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=203112k,mode=755)

On install the unit is enabled (postinst has the dusual dh_installsystemd snippet), but sadly it stays disabled. Even after reboot, despite the enabled config it stayed disabled.

To be clear once started (manually for now) it works fine
$ sudo systemctl start run-qemu.mount
$ systemctl status run-qemu.mount
● run-qemu.mount - Allow noexec to for late qemu module load after upgrades
     Loaded: loaded (/lib/systemd/system/run-qemu.mount; disabled; vendor preset: enabled)
     Active: active (mounted) since Wed 2021-01-27 20:09:45 UTC; 1s ago
      Where: /run/qemu
       What: tmpfs
      Tasks: 0 (limit: 2338)
     Memory: 24.0K
     CGroup: /system.slice/run-qemu.mount

Jan 27 20:09:45 h-qemu-modules systemd[1]: Mounting Allow noexec to for late qemu module load after upgrades...
Jan 27 20:09:45 h-qemu-modules systemd[1]: Mounted Allow noexec to for late qemu module load after upgrades.

$ mount | grep run
tmpfs on /run type tmpfs (rw,nosuid,nodev,noexec,relatime,size=203112k,mode=755)
tmpfs on /run/qemu type tmpfs (rw,nosuid,nodev,relatime,mode=755)

And a reinstall now places files in there as it did before:

$ ls -laFd /var/run/qemu; ls -laF /var/run/qemu; ls -laF /var/run/qemu/*
drwxr-xr-x 3 root root 60 Jan 27 20:11 /var/run/qemu/
total 0
drwxr-xr-x 3 root root 60 Jan 27 20:11 ./
drwxr-xr-x 30 root root 880 Jan 27 20:09 ../
drwxr-xr-x 2 root root 120 Jan 27 20:11 Debian_1_5.2+dfsg-3ubuntu2~ppa2/
total 164
drwxr-xr-x 2 root root 120 Jan 27 20:11 ./
drwxr-xr-x 3 root root 60 Jan 27 20:11 ../
-rw-r--r-- 1 root root 38632 Jan 27 12:45 block-curl.so
-rw-r--r-- 1 root root 45160 Jan 27 12:45 block-iscsi.so
-rw-r--r-- 1 root root 35912 Jan 27 12:45 block-rbd.so
-rw-r--r-- 1 root root 40136 Jan 27 12:45 block-ssh.so

Seems I'm cursed with dh_installsystemd magice recently, maybe the .mount behaves differently in dh*. In any case the postinst really has no start section for it, so it can't work yet.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

open-vm-tools-desktop
dh_installsystemd -popen-vm-tools-desktop --restart-after-upgrade --no-stop-on-upgrade run-vmblock

=> Maintscripts
# Automatically added by dh_installsystemd/13.3.1ubuntu1
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
        # This will only remove masks created by d-s-h on package removal.
        deb-systemd-helper unmask 'run-vmblock\x2dfuse.mount' >/dev/null || true

        # was-enabled defaults to true, so new installations run enable.
        if deb-systemd-helper --quiet was-enabled 'run-vmblock\x2dfuse.mount'; then
                # Enables the unit on first installation, creates new
                # symlinks on upgrades if the unit file has changed.
                deb-systemd-helper enable 'run-vmblock\x2dfuse.mount' >/dev/null || true
        else
                # Update the statefile to add new symlinks (if any), which need to be
                # cleaned up on purge. Also remove old symlinks.
                deb-systemd-helper update-state 'run-vmblock\x2dfuse.mount' >/dev/null || true
        fi
fi
# End automatically added section
# Automatically added by dh_installsystemd/13.3.1ubuntu1
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
        if [ -d /run/systemd/system ]; then
                systemctl --system daemon-reload >/dev/null || true
                if [ -n "$2" ]; then
                        _dh_action=restart
                else
                        _dh_action=start
                fi
                deb-systemd-invoke $_dh_action 'run-vmblock\x2dfuse.mount' >/dev/null || true
        fi
fi
# End automatically added section

Qemu as I tried it atm:

dh_installsystemd -a -pqemu-system-common --restart-after-upgrade --no-stop-on-upgrade --name=run-qemu.mount

Only gets rendered into
# Automatically added by dh_installsystemd/13.3.1ubuntu1
if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
        if deb-systemd-helper debian-installed 'run-qemu.mount'; then
                # This will only remove masks created by d-s-h on package removal.
                deb-systemd-helper unmask 'run-qemu.mount' >/dev/null || true

                if deb-systemd-helper --quiet was-enabled 'run-qemu.mount'; then
                        # Create new symlinks, if any.
                        deb-systemd-helper enable 'run-qemu.mount' >/dev/null || true
                fi
        fi

        # Update the statefile to add new symlinks (if any), which need to be cleaned
        # up on purge. Also remove old symlinks.
        deb-systemd-helper update-state 'run-qemu.mount' >/dev/null || true
fi
# End automatically added section

The start section is missing, but why?

Both are compat level 12, the commands are the same ... hmmm

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

This is as open-vm-tools, there it works :-/
--restart-after-upgrade --no-stop-on-upgrade

But no matter if I use either of the following for dh_installsystemd:
"--restart-after-upgrade --no-stop-on-upgrade"
"--no-stop-on-upgrade"
""

I always end up with:
/var/lib/dpkg/info/qemu-system-common.postinst: if deb-systemd-helper debian-installed 'run-qemu.mount'; then
/var/lib/dpkg/info/qemu-system-common.postinst: deb-systemd-helper unmask 'run-qemu.mount' >/dev/null || true
/var/lib/dpkg/info/qemu-system-common.postinst: if deb-systemd-helper --quiet was-enabled 'run-qemu.mount'; then
/var/lib/dpkg/info/qemu-system-common.postinst: deb-systemd-helper enable 'run-qemu.mount' >/dev/null || true
/var/lib/dpkg/info/qemu-system-common.postinst: deb-systemd-helper update-state 'run-qemu.mount' >/dev/null || true
/var/lib/dpkg/info/qemu-system-common.postrm: deb-systemd-helper mask 'qemu-kvm.service' 'run-qemu.mount' >/dev/null || true
/var/lib/dpkg/info/qemu-system-common.postrm: deb-systemd-helper purge 'qemu-kvm.service' 'run-qemu.mount' >/dev/null || true
/var/lib/dpkg/info/qemu-system-common.postrm: deb-systemd-helper unmask 'qemu-kvm.service' 'run-qemu.mount' >/dev/null || true

And that is neither good after install (no start action)
nor after reboot as it stays disabled:
 Loaded: loaded (/lib/systemd/system/run-qemu.mount; disabled; vendor preset: enabled)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Here how open-vm-tools looks like:
/var/lib/dpkg/info/open-vm-tools-desktop.postinst: deb-systemd-helper unmask 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postinst: if deb-systemd-helper --quiet was-enabled 'run-vmblock\x2dfuse.mount'; then
/var/lib/dpkg/info/open-vm-tools-desktop.postinst: deb-systemd-helper enable 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postinst: deb-systemd-helper update-state 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postinst: deb-systemd-invoke $_dh_action 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postrm: deb-systemd-helper mask 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postrm: deb-systemd-helper purge 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.postrm: deb-systemd-helper unmask 'run-vmblock\x2dfuse.mount' >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.prerm: deb-systemd-invoke stop run-vmblock\\x2dfuse.mount >/dev/null || true
/var/lib/dpkg/info/open-vm-tools-desktop.prerm: deb-systemd-invoke stop 'run-vmblock\x2dfuse.mount' >/dev/null || true

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

     Loaded: loaded (/lib/systemd/system/run-vmblock\x2dfuse.mount; enabled; vendor preset: enabled)

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

LOL+ I had to major breakthroughs on this:
1. I found the right way in d/rules to get the .mount unit started
2. I had a great discussion about "the other POV" on this [1] and I must say that I agree.
   As much as this can be a comfort function it can also be
   a) less reasons to finally restart into upgraded code
   b) leave security vulnerable code around

For that I think we really want to make this available, but also NOT enabled by default.
As an opt-in that makes sense.

Current plan - I'll prep changes along this one here that does:
- install the .mount but NOT start/enable it (the admin has to opt in)
  The admin also can pick any other way he prefers to make /run/qemu not have noexec
- define a /etc/.. place to enable this feature, and otherwise have the postrm not even
  copy the old bits.

[1]:

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

[1]: is debian-devel of this morning which seems ot have no public logs :-/

Not sure if I'm supposed to post them then

Revision history for this message
Dan Streetman (ddstreet) wrote :

This sounds very complicated. Are you *sure* using a simple tmpfiles.d approach wouldn't be better?

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi Dan,
the opt-in we want anyway - nothing of that is tied to the question of .mount vs tmpfiles

We had identified some drawback in tmpfiles that made us chose .mount, but maybe that changed given the recent discussions/insights. I'll re-read out old discussion why we dis-qualified tmpfiles and if it is - nowadays - the better option I'll reconsider this.

P.S. also part of the complexity that is flowing by here is the lack of many existing examples for a .mount unit, so there was a bit of experimentation needed. The final result isn't as complex.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

@Dan - from the discussion we had the outcome was that tmpfiles can only create directories and set ownership. At the same time the path is set (per upstream agreement cross distros) and also due to apparmor confinement no symlink magic will help. But the issue we ahve here is that we need to have /run/qemu to be NOT noexec which /run in many cases is by default.
I haven't seen any comeback of a tmpfiles solution as those limitations were not overcome.

If you strip out all the trial and error I had on this bug then it is just:
1. Victor told me we need "exec", he is right
2. Discussion with more developers showed that this feature, although nice - should
   really not be default enabled (but we are fine to make it a comfortable opt-in).
3. I'm prepping a change that fulfills
   #1 with a .mount unit
   #2 with a config file and the .mount being default disabled

The suggested config file would be:
/etc/default/qemu-block-extra-upgrade-backup

The files there usually are == package name, but this is a very special case so just naming it qemu-block-extra seems wrong. Starting with the package name, but having a suffix is what I'd go for until review happens.

Revision history for this message
Dan Streetman (ddstreet) wrote :

> the opt-in we want anyway

major nak from me. this can't be opt-in. Can you explain the concern?

> from the discussion we had the outcome was that tmpfiles can only create directories
> and set ownership

that's not true

> apparmor confinement no symlink magic will help

I'm not thinking of symlinks, and any apparmor changes needed would be the same for /run storage or tmpfiles approach

Revision history for this message
Dan Streetman (ddstreet) wrote :

Another completely different alternative approach might be for us to see if upstream qemu is willing to simply open all the module files when qemu starts, and leave the fd open until exit.

That way even if the module files are removed, any still-running qemu process(es) would still have an open fd to them and (at least on UNIX systems) should be able to load them, since the kernel won't actually fully remove them until all open descriptors are closed.

I haven't tested that and I'm not sure if there are possible issues with mmaping removed files, but in theory it should work.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote : Re: [Bug 1913421] Re: Load of pre-upgrade qemu modules needs to avoid noexec

On Thu, Feb 11, 2021 at 2:45 PM Dan Streetman
<email address hidden> wrote:
>
> Another completely different alternative approach might be for us to see
> if upstream qemu is willing to simply open all the module files when
> qemu starts, and leave the fd open until exit.

We've had that discussion the first time it came up, but that wasn't
an approach anyone likes.
It has too many bad attributes:
- keeping files open that are removed is considered not-good
- bloating the active binary is considered very bad and by mapping all
that would happen
- There are more awkward cases, like starting guests, then installing
qemu-block-extra later and then hot-plugging
   Valid but not working with this approach.

> That way even if the module files are removed, any still-running qemu
> process(es) would still have an open fd to them and (at least on UNIX
> systems) should be able to load them, since the kernel won't actually
> fully remove them until all open descriptors are closed.
>
> I haven't tested that and I'm not sure if there are possible issues with
> mmaping removed files, but in theory it should work.
>
> ** Merge proposal linked:
> https://code.launchpad.net/~ddstreet/ubuntu/+source/qemu/+git/qemu/+merge/397904
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1913421
>
> Title:
> Load of pre-upgrade qemu modules needs to avoid noexec
>
> Status in qemu package in Ubuntu:
> In Progress
> Status in qemu source package in Bionic:
> Confirmed
> Status in qemu source package in Focal:
> Confirmed
> Status in qemu source package in Groovy:
> Confirmed
>
> Bug description:
> This is a continuation of bug 1847361.
>
> Since that is in Ubuntu and Debian we are:
> - correctly saving the modules to those paths in /var/run/qemu.
> - qemu tries to load from that path as fallback
> - that works fine in containers running qemu/kvm
>
> But there is an issue on non-container systems as /run usually is like
> this:
>
> tmpfs on /run type tmpfs
> (rw,nosuid,nodev,noexec,relatime,size=3274920k,mode=755)
>
> The important bit here is the "noexec" which is intentional (for
> security reasons), but prevents the loading of shared objects from
> that path.
>
> The path is good for many reasons (it is auto-cleaned, upstream and
> Distros agreed to this one path, ...). Moving it to other places also
> quite likely might have unpredictable options.
>
> In a discussion between Victor (thanks for all the pushign and inpot
> on this) and Marc (security POV) we have come to a solution that will
> make just the subpath that is owned by qemu to not have noexec set.
>
> This bug shall track preparing this fix for Debian / Ubuntu and the
> latter SRu considerations on the same.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1913421/+subscriptions

--
Christian Ehrhardt
Staff Engineer, Ubuntu Server
Canonical Ltd

Revision history for this message
Dan Streetman (ddstreet) wrote :

@paelzer when you sru for this bug, can you include the patches from bug 1887535 and bug 1887823 also? I have MR for them linked from the bugs

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Yes Dan, I'd include those as they'd ride a change that makes the upload qualify for an SRU.
But first we need to agree/complete it and also another set of CVE uploads needs to pass.

Revision history for this message
Markus Schade (lp-markusschade) wrote :

Are there any plans to go forward with any of the above mentioned solutions?
Currently I either have to migrate off all guests before a qemu upgrade or set up a tmpfs mount with exec on /run/qemu.

Happy to help testing.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi Markus,
yeah the intention is to pick one once we agree with Debian which one to not go back&forth too often. That was discussion was stuck for a while but recently unblocked.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers