libvirt-qemu apparmor profile doesn't allow locking of AAVMF firmware

Bug #1989078 reported by Paride Legovini
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
libvirt
Fix Released
Unknown
libvirt (Ubuntu)
Fix Released
Undecided
Unassigned
Focal
Fix Committed
Undecided
Christian Ehrhardt 
Jammy
Fix Released
Undecided
Christian Ehrhardt 

Bug Description

[ Impact ]

 * Qemu on arm64 changed behavior now trying to ensure exclusivity on the
   AAVM files used by OVMF. That is stopped by apparmor and needs to be
   allowed to function again (as it used to prior to jammy before the
   locking took place).

 * The fix does not change the code, instead it changes the rule for
   the specific path in the guest-associated apparmor rule and
   adds "k" (=locking) to it.

[ Test Plan ]

 * On arm64 you can try to spawn an OVMF using test system
   (from the upstream report):
   $ virt-install --name test --ram 1024 --vcpus 2 --disk size=16 --location https://ftp.debian.org/debian/dists/bullseye/main/installer-arm64/ --osinfo debian11 --graphics none --tpm none --boot uefi

   Without the fix that will emit an apparmor denial like:

   apparmor="DENIED" operation="file_lock" profile="libvirt-2c81aaf3-2e3d-44a3-9acb-0e1e3c9bd54c" name="/usr/share/AAVMF/AAVMF_CODE.fd" pid=323963 comm="qemu-system-aa>

   With the fix that apparmor denial will not occur and it reaches further
   stages of guest initialization.

For tests on focal only a subset trigger with the qemu in focal, but any self-build or backport, like the one from https://launchpad.net/~canonical-server/+archive/ubuntu/server-backports will use the extended locking that triggers the problem on ovmf/aavmf resources as well. So consider adding those (but not the libvirt from that PPA as it already has the fix) to reproduce.

[ Where problems could occur ]

 * Since it "only" changes apparmor rules and in doing so only "widens"
   what is allowed there should be not many problems, those I can think of
   are
   - Conffile churn: this changes a conffile and despit all documentation
     pointers and hints to use a local override some people have changes.
     So we might override some of those or at least trigger conffile
     prompts.
   - Usually restricting apparmor is more risky than opening it up. The
     one pattern that could happen is that in some places something
     "expected to fail will now work. But that is the purpose of the fix.
   - gladly AAVMF is only used for arm, so other platforms should be
     "even more unaffected"

[ Other Info ]

 * a return of bug 1709818 but for a different file type

---

[Filing https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1709818/comments/8 as a separate bug report. Thanks J. S. Seldenthuis (jseldent) for reporting it.]

Qemu tries to lock the uefi firmware image during vm creation, but the libvirt-supplied apparmor profile prevents this for the AAVMF firmware, and qemu fails with:

  qemu-system-arm: Failed to lock byte 100

The solution is adding the "k" flag to the relevant apparmor profile. This is fixed upstream by this commit:

https://gitlab.com/libvirt/libvirt/-/commit/2b98d5d91d95087d8a96d6450fa96414ed05ba5c

which is already included in the libvirt version in Kinetic.

Related branches

Paride Legovini (paride)
Changed in libvirt (Ubuntu Jammy):
status: New → Triaged
Changed in libvirt (Ubuntu):
status: New → Fix Released
tags: added: server-todo
Changed in libvirt:
status: Unknown → Fix Released
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks @J.S. indeed for the report.
And thanks Paride for the re-triage into a new bug - that was just right.

It is indeed the same as in the past and fixed the same way but for a new file path.
Therefore we do not need much external checks to wait for.

And gladly since it is in v8.4.0 is is already fixed in Kinetic and we can start with the SRU right away.
I backported the fix and provided a PPA with it for testing if you want.

Prepared a fix here for test and review:
- PPA: https://launchpad.net/~paelzer/+archive/ubuntu/lp-1989078-aavmf-locking
- MP: https://launchpad.net/~paelzer/+archive/ubuntu/lp-1989078-aavmf-locking

Changed in libvirt (Ubuntu Jammy):
assignee: nobody → Christian Ehrhardt  (paelzer)
description: updated
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

FYI: can be tested and verified on Canonistack arm64 instance.

As expected:
$ virt-install --name test --ram 1024 --vcpus 2 --disk size=16 --location https://ftp.debian.org/debian/dists/bullseye/main/installer-arm64/ --osinfo debian11 --graphics none --tpm none --boot uefi
...
ERROR internal error: process exited while connecting to monitor: 2022-09-12T06:29:45.846337Z qemu-system-aarch64: Failed to lock byte 100
Domain installation does not appear to have been successful.

In Journal:
Sep 12 06:29:45 cpaelzer-arm64-libvirt-aavmf-1989078 kernel: audit: type=1400 audit(1662964185.843:40): apparmor="DENIED" operation="file_lock" profile="libvirt-10bfeb48-3d08-4c92-a959-c1f27b241978" name="/usr/share/AAVMF/AAVMF_CODE.fd" pid=6034 comm="qemu-system-aar" requested_mask="k" denied_mask="k" fsuid=64055 ouid=0
Sep 12 06:29:45 cpaelzer-arm64-libvirt-aavmf-1989078 libvirtd[4970]: internal error: qemu unexpectedly closed the monitor: 2022-09-12T06:29:45.846337Z qemu-system-aarch64: Failed to lock byte 100

With the fix installed we get into an UEFI shell in the guest, there then is a lack of further progress but that is something else (due to the trivial setup used). The problem reported here is fixed by the change as expected.

Uploading as Jammy SRU ...

Changed in libvirt (Ubuntu Jammy):
status: Triaged → In Progress
Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello Paride, or anyone else affected,

Accepted libvirt into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/8.0.0-1ubuntu7.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in libvirt (Ubuntu Jammy):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-jammy
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (20.9 KiB)

This can also be tried with emulation on foreign arch (which is what i tried not having a arm64 system atm). Just add --arch aarch64

Without the fix:

ubuntu@j:~$ virt-install --name test --ram 256 --vcpus 1 --disk size=1 --location https://ftp.debian.org/debian/dists/bullseye/main/installer-arm64/ --osinfo debian11 --graphics none --tpm none --boot uefi --arch aarch64
WARNING Requested memory 256 MiB is less than the recommended 1024 MiB for OS debian11

Starting install...
Retrieving 'linux' | 25 MB 00:00:03 ...
Retrieving 'initrd.gz' | 28 MB 00:00:03 ...
Allocating 'virtinst-chmmf_yx-linux' | 0 B 00:00:00 ...
Transferring 'virtinst-chmmf_yx-linux' | 0 B 00:00:00 ...
Allocating 'virtinst-o3iokdvl-initrd.gz' | 0 B 00:00:00 ...
Transferring 'virtinst-o3iokdvl-initrd.gz' | 0 B 00:00:00 ...
Allocating 'test-1.qcow2' | 0 B 00:00:00 ...
Removing disk 'test-1.qcow2' | 0 B 00:00:00
ERROR internal error: process exited while connecting to monitor: 2022-10-05T08:21:35.575991Z qemu-system-aarch64: Failed to lock byte 100
Domain installation does not appear to have been successful.
If it was, you can restart your domain by running:
  virsh --connect qemu:///system start test
otherwise, please restart your installation.

Log:
Oct 05 08:21:35 j libvirtd[19092]: internal error: qemu unexpectedly closed the monitor: 2022-10-05T08:21:35.575991Z qemu-system-aarch64: Failed to lock byte 100
Oct 05 08:21:35 j libvirtd[19092]: internal error: process exited while connecting to monitor: 2022-10-05T08:21:35.575991Z qemu-system-aarch64: Failed to lock byte 100

Installing from proposed

ubuntu@j:~$ sudo apt upgrade
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
Calculating upgrade... Done
The following NEW packages will be installed:
  linux-headers-5.15.0-50 linux-headers-5.15.0-50-generic linux-image-5.15.0-50-generic linux-modules-5.15.0-50-generic python3-magic
The following packages will be upgraded:
  binutils binutils-common binutils-x86-64-linux-gnu cloud-init grub-efi-amd64-bin grub-efi-amd64-s...

tags: added: verification-done verification-done-jammy
removed: verification-needed verification-needed-jammy
Revision history for this message
João Pedro Seara (jpseara) wrote :

Just tested this fix successfully!

Thanks a lot.

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

This bug was fixed in the package libvirt - 8.0.0-1ubuntu7.2

---------------
libvirt (8.0.0-1ubuntu7.2) jammy; urgency=medium

  * d/p/u/lp-1989078-apparmor-Allow-locking-AAVMF-firmware.patch: allow arm64
    to lock its OVMF resources (LP: #1989078)

 -- Christian Ehrhardt <email address hidden> Thu, 08 Sep 2022 12:00:39 +0200

Changed in libvirt (Ubuntu Jammy):
status: Fix Committed → Fix Released
Revision history for this message
Timo Aaltonen (tjaalton) wrote : Update Released

The verification of the Stable Release Update for libvirt has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

tags: removed: server-todo
Revision history for this message
Christian Ehrhardt  (paelzer) wrote (last edit ):

We haven't considered Focal back then.
But this report [1] made me realize that it is there just as much of a problem.
The solution is simple, proven and should face no unexpected hiccups given what/how it does.
Adding a Focal bug task.

In Focal also [2] has not yet landed, but would be needed (and ok to be applied) just as much.
Some of these might only trigger with when used with newer qemu versions (which lock a few more file types), but that is not something we want to restrict by holding it back.

[1]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1709818/comments/10
[2]: https://gitlab.com/libvirt/libvirt/-/commit/7aec69b7fb9d0cfe8b7203473764c205b28d2905

Changed in libvirt (Ubuntu Focal):
status: New → Triaged
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
tags: added: server-todo
Revision history for this message
Chris Halse Rogers (raof) wrote : Please test proposed package

Hello Paride, or anyone else affected,

Accepted libvirt into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libvirt/6.0.0-0ubuntu8.17 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in libvirt (Ubuntu Focal):
status: Triaged → Fix Committed
tags: added: verification-needed verification-needed-focal
removed: verification-done
Robie Basak (racb)
Changed in libvirt (Ubuntu Focal):
assignee: nobody → Christian Ehrhardt  (paelzer)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I'm testing this on an rpi4 (using arm64).

root@faavmf:~# virt-install --name test --ram 512 --vcpus 2 --disk size=2 --location https://ftp.debian.org/debian/dists/bullseye/main/installer-arm64/ --graphics none --tpm none --boot uefi
...

Now this is a bit tricky now, as it works fine with the fix.

I checked the guest definition and it has what we expected it to have to make it trigger:
    <loader readonly='yes' type='pflash'>/usr/share/AAVMF/AAVMF_CODE.fd</loader>

The qemu being the guest has it open:
root@faavmf:~# ll /proc/7904/fd/12
lr-x------ 1 libvirt-qemu kvm 64 Feb 2 10:54 /proc/7904/fd/12 -> /usr/share/AAVMF/AAVMF_CODE.fd

And the tricky part is that it also works fine without the fix!
You might think - that seems wrong, I thought so as well - as I said, it is tricky.

The locking of images, and later of boot resources was added to qemu over time.
And using the original qemu of focal didn't lock the aavmf path yet.

But if one would be using a newer qemu then this would happen (An example might be using server-backports, but TBH the libvirt from there would already have the fix).

We want to be open to other use-cases, the rule is considered safe, and applied upstream and in later releases. Users can use other qemu builds, and while we do not support them, we do not artificially block them either.

Also I continued testing the new version and it is fine.

So where are we:
1. the new libvirt works fine and allows the access we got reported
2. the old libvirt works also fine in any common config (but would not allow the access)

I'd call this verification-done (it does work, and the new access is allowed).
But at the same time since this is such a edge case of edge cases I think it would be wasting bandwidth and patience to release that.
Hence i'd also mark it block-proposed to only be released when riding on top of any other follow on fix that is worth triggering downloads on millions of machines.

If anyone is affected and can show that it is more real and more urgent, please describe the case to demonstrate why.

[1]: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1709818/comments/10

tags: added: block-proposed block-proposed-focal verification-done verification-done-focal
removed: verification-needed verification-needed-focal
tags: removed: server-todo
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.