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

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

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 ...

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.

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...

Just tested this fix successfully!

Thanks a lot.

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

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.

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

Prepared the fix for focal:
- PPA test build: https://launchpad.net/~paelzer/+archive/ubuntu/lp-1989078-aavmf-locking
- MR to review: https://code.launchpad.net/~paelzer/ubuntu/+source/libvirt/+git/libvirt/+merge/435343

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.

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

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.18 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.

We now have a follow-up SRU. Removing the block-proposed tags.

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.20 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.

Updating the verification tags for the new version,
agreeing with Christian's assessment in comment 12,
and verifying that the changes are still included:

$ pull-lp-debs -a arm64 libvirt-daemon-system focal 6.0.0-0ubuntu8.20
$ dpkg-deb -x libvirt-daemon-system_6.0.0-0ubuntu8.20_arm64.deb deb
$ fgrep -i -e '/usr/share/ovmf/**' -e 'owner /var/lib/libvirt/qemu/nvram/*_VARS' deb/etc/apparmor.d/abstractions/libvirt-qemu
  /usr/share/ovmf/** rk,
  /usr/share/OVMF/** rk,
  owner /var/lib/libvirt/qemu/nvram/*_VARS.fd rwk,
  owner /var/lib/libvirt/qemu/nvram/*_VARS.ms.fd rwk,
$

This bug was fixed in the package libvirt - 6.0.0-0ubuntu8.20

---------------
libvirt (6.0.0-0ubuntu8.20) focal; urgency=medium

  * d/p/u/lp2059272-2-qemu-Wait-qemuProcessReconnect-threads-in-cleanup.patch:
    Remove patch. It is not possible to wait for qemuProcessReconnect()
    in cleanup: it talks to QEMU monitor, which blocks on replies from
    event loop, but it's already stopped at cleanup, delaying shutdown.

  * d/p/u/lp2059272-2-qemu-Do-not-save-XML-in-shutdown-on-init.patch:
    Instead of waiting at cleanup for threads which might be blocked
    thus would _not even reach_ the function that causes the problem,
    just skip that function if it is _actually reached_ while daemon
    shutdown is in progress. That is in the init path and would just
    run again anyway the next time libvirtd is started (LP: #2059272)

  * NOTE: This package contains the changes from 6.0.0-0ubuntu8.18 and
    6.0.0-0ubuntu8.17 in focal-proposed (with symbolic changelog entry)
    superseded by 6.0.0-0ubuntu8.19 in focal-security.

libvirt (6.0.0-0ubuntu8.20~ubuntu8.18) focal; urgency=medium

  * d/p/u/lp2059272-1-qemu-Fix-potential-crash-during-driver-cleanup.patch:
    On QEMU driver cleanup, release (stop) the worker thread pool _first_,
    before other data used by possibly running worker threads (LP: #2059272)

  * d/p/u/lp2059272-2-qemu-Wait-qemuProcessReconnect-threads-in-cleanup.patch:
    On QEMU driver cleanup, also wait for qemuProcessReconnect() threads,
    as they are independent of the worker thread pool. (LP: #2059272)
    Focal needs this as it has no .stateShutdownWait() callback yet.
    (The wait timeout is set in LIBVIRT_QEMU_STATE_CLEANUP_WAIT_TIMEOUT:
     -1 = wait indefinitely; 0 = do not wait; N = wait up to N seconds.)

libvirt (6.0.0-0ubuntu8.20~ubuntu8.17) focal; urgency=medium

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

 -- Mauricio Faria de Oliveira <email address hidden> Tue, 16 Apr 2024 14:20:13 -0300