Merge ~mustafakemalgilor/ubuntu/+source/systemd:systemd-fix-lp1978079-efi-pstore-not-cleared-focal into ~ubuntu-core-dev/ubuntu/+source/systemd:ubuntu-focal

[Mustafa Kemal Gilor]

Backported a fix from upstream to focal for the LP issue #1978079:

      * d/p/lp1978079-efi-pstore-not-cleared-on-boot.patch: pstore: Run
        after modules are loaded. Thanks to Alexander Graf
        <email address hidden>. (LP: #1978079)

Please tag & sponsor.

(See backport merge request for jammy:
https://code.launchpad.net/~mustafakemalgilor/ubuntu/+source/systemd/+git/systemd/+merge/424958)

Steps to reproduce/test:

# In order to be able to reproduce this issue, the system must have EFI-backed pstore.
# To check which kind of backend that pstore, use `cat /sys/module/pstore/parameters/backend`
# If it says `efi`, the steps below are applicable. Otherwise, find an environment that has
# EFI backed pstore.

# Enable the pstore service. This service is supposed to move the data in /sys/fs/pstore
# to the `/var/lib/systemd/pstore` path on boot.
systemctl enable systemd-pstore.service # (or can be vendor enabled)

# Crash the kernel
echo 1 > /proc/sys/kernel/sysrq
echo 1 > /proc/sys/kernel/panic # this is usually set to zero, causing kernel to loop over the panic and freeze
echo "c" > /proc/sysrq-trigger

# The system will reboot itself. Check `/sys/fs/pstore` path first:
ls /sys/fs/pstore # The path should not be empty, which means the systemd-pstore has failed to do its' job
ls /var/lib/systemd/pstore # The path should be empty.

# Apply the fix
sudo add-apt-repository ppa:mustafakemalgilor/lp-1978079-1
sudo apt upgrade

# Crash the kernel
echo 1 > /proc/sys/kernel/sysrq
echo 1 > /proc/sys/kernel/panic # this is usually set to zero, causing kernel to loop over the panic and freeze
echo "c" > /proc/sysrq-trigger

# The system will reboot itself. Check `/sys/fs/pstore` path first:
ls /sys/fs/pstore # The path should be empty
ls /var/lib/systemd/pstore # The path should not be empty

Package Test Results:

autopkgtest [13:08:54]: test systemd-fsckd: [-----------------------
SKIP: root file system is being checked by initramfs already
autopkgtest [13:08:55]: test systemd-fsckd: -----------------------]
systemd-fsckd SKIP exit status 77 and marked as skippable
autopkgtest [13:08:56]: test systemd-fsckd: - - - - - - - - - - results - - - - - - - - - -
autopkgtest [13:08:56]: @@@@@@@@@@@@@@@@@@@@ summary
timedated PASS
hostnamed PASS
localed-locale PASS
localed-x11-keymap PASS
logind PASS
unit-config PASS
storage PASS
networkd-test.py PASS
build-login PASS
boot-and-services PASS
udev PASS
root-unittests PASS
upstream PASS
boot-smoke PASS
systemd-fsckd SKIP exit status 77 and marked as skippable

af8e899... by Mustafa Kemal Gilor on 2022-06-20

* d/p/lp1978079-efi-pstore-not-cleared-on-boot.patch: pstore: Run after
  modules are loaded. Thanks to Alexander Graf <email address hidden>.
  (LP: #1978079)

c42c169... by Mustafa Kemal Gilor on 2022-06-20

updated changelog

[Robie Basak (racb)]

Normally the Foundations team reviews systemd changes. Is there a particular reason you requested reviews from the Server Team please?

[Mustafa Kemal Gilor (mustafakemalgilor)]

There is no particular reason. This is my first merge proposal and I've followed the Ubuntu Maintainer's Handbook (https://github.com/canonical/ubuntu-maintainers-handbook/blob/main/MergeProposal.md) for structuring my MP. It seems like either I've missed something, or the handbook is outdated.

[Lukas Märdian (slyon)]

Thank you very much! This is quality work and LGTM.

Good work on the detailed reproducer/testcase and thanks for executing autopkgtest ahead of time!

I will probably re-write the d/changelog file a bit, to bring it in line with our "gbp dch" (git-buildpackage) workflow. Other than that, I'll merge it as is.

review: Approve

[Robie Basak (racb)]

> There is no particular reason. This is my first merge proposal and I've followed the Ubuntu Maintainer's Handbook (https://github.com/canonical/ubuntu-maintainers-handbook/blob/main/MergeProposal.md) for structuring my MP. It seems like either I've missed something, or the handbook is outdated.

Ah OK, thanks for the feedback. The handbook was written by the server team with the idea that it'd also be useful to others. So some parts are currently server-specific, such as "how to find a reviewer", on the assumption that it's a package that the server team looks after.

I'm separately working on processes to help widen the usefulness of this, such as to integrate with the general sponsorship queue. So I'll carry on with that and update the docs when done.

[Mustafa Kemal Gilor (mustafakemalgilor)]

> Ah OK, thanks for the feedback. The handbook was written by the server team with the idea that it'd also be useful to others. So some parts are currently server-specific, such as "how to find a reviewer", on the assumption that it's a package that the server team looks after.

Ah, I had no idea :)

> I'm separately working on processes to help widen the usefulness of this, such as to integrate with the general sponsorship queue. So I'll carry on with that and update the docs when done.

Thanks for considering that; it would be super helpful for the newcomers. Even in the current state, the handbook is pretty solid, and I'd say that I have learned a great deal about the SRU process by following it. The SRU process has many details, and it is *very* easy to get lost. Having such a guide helps a lot.

diff --git a/debian/changelog b/debian/changelog
index c1b9069..ba53e27 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,6 @@
 systemd (245.4-4ubuntu3.18) UNRELEASED; urgency=medium
 
+  [ Nick Rosbrook ]
   * core: make sure we don't get confused when setting TERM for a tty fd
     (LP: #1959475)
     File: debian/patches/lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
@@ -9,6 +10,11 @@ systemd (245.4-4ubuntu3.18) UNRELEASED; urgency=medium
     File: debian/patches/lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch
     https://git.launchpad.net/~ubuntu-core-dev/ubuntu/+source/systemd/commit/?id=1f063541e44f6ff1a6904676d4264a2e49a09594
 
+  [ Mustafa Kemal GILOR ]
+  * d/p/lp1978079-efi-pstore-not-cleared-on-boot.patch: pstore: Run after
+    modules are loaded. Thanks to Alexander Graf <[e-mail]>.
+    (LP: #1978079)
+
  -- Nick Rosbrook <[e-mail]>  Fri, 29 Apr 2022 15:03:31 -0400
 
 systemd (245.4-4ubuntu3.17) focal; urgency=medium
diff --git a/debian/patches/lp1978079-efi-pstore-not-cleared-on-boot.patch b/debian/patches/lp1978079-efi-pstore-not-cleared-on-boot.patch
new file mode 100644
index 0000000..2b282d0
--- /dev/null
+++ b/debian/patches/lp1978079-efi-pstore-not-cleared-on-boot.patch
@@ -0,0 +1,41 @@
+From 4dec0b605b378b1a3d54bd738130cc45ff745673 Mon Sep 17 00:00:00 2001
+From: Alexander Graf <[e-mail]>
+Date: Thu, 9 Jun 2022 16:20:43 +0200
+Subject: [PATCH] pstore: Run after modules are loaded
+
+The systemd-pstore service takes pstore files on boot and transfers them
+to disk. It only does it once on boot and only if it finds any. The typical
+location of the pstore on modern systems is the UEFI variable store.
+
+Most distributions ship with CONFIG_EFI_VARS_PSTORE=m. That means, the
+UEFI variable store is only available on boot after the respective module
+is loaded.
+
+In most situations, the pstore service gets loaded before the UEFI pstore,
+so we don't get to transfer logs. Instead, they accumulate, filling up the
+pstore over time, potentially breaking the UEFI variable store.
+
+Let's add a service dependency on any kernel module that can provide a
+pstore to ensure we only scan for pstate after we can actually see pstate.
+
+I have seen live occurences of systems breaking because we did not erase
+the pstates and ran out of UEFI nvram space.
+
+Fixes https://github.com/systemd/systemd/issues/18540
+---
+ units/systemd-pstore.service.in | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/units/systemd-pstore.service.in b/units/systemd-pstore.service.in
+index 848e311e9642..86de30ad4a72 100644
+--- a/units/systemd-pstore.service.in
++++ b/units/systemd-pstore.service.in
+@@ -15,6 +15,8 @@ ConditionVirtualization=!container
+ DefaultDependencies=no
+ Conflicts=shutdown.target
+ Before=sysinit.target shutdown.target
++After=modprobe@efi_pstore.service [e-mail] modprobe@chromeos_pstore.service [e-mail] modprobe@pstore_zone.service modprobe@pstore_blk.service
++Wants=modprobe@efi_pstore.service [e-mail] modprobe@chromeos_pstore.service [e-mail] modprobe@pstore_zone.service modprobe@pstore_blk.service
+ 
+ [Service]
+ Type=oneshot
diff --git a/debian/patches/series b/debian/patches/series
index 6441bb3..56ece03 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -177,3 +177,4 @@ hwdb-Add-mic-mute-key-mapping-for-HP-Elite-x360.patch
 lp1966179-add-more-hp-dmi-to-unblock-intel-hid-event.patch
 lp1959475-core-make-sure-we-don-t-get-confused-when-setting-TERM-fo.patch
 lp1966800-shared-calendarspec-when-mktime-moves-us-backwards-jump-f.patch
+lp1978079-efi-pstore-not-cleared-on-boot.patch