| Message ID | 20250107213129.28454-3-James.Bottomley@HansenPartnership.com |
|---|---|
| State | New |
| Headers | show |
| Series | efivarfs: fix hibernation problem with EFI variables | expand |
Hi James, On 07/01/2025 21:31, James Bottomley wrote: > Hibernation allows other OSs to boot and thus the variable state might > be altered by the time the hibernation image is resumed. Resync the > variable state by looping over all the dentries and update the size > (in case of alteration) delete any which no-longer exist. Finally, > loop over all efi variables creating any which don't have > corresponding dentries. > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> > --- > fs/efivarfs/internal.h | 3 +- > fs/efivarfs/super.c | 151 ++++++++++++++++++++++++++++++++++++++++- > fs/efivarfs/vars.c | 5 +- > 3 files changed, 155 insertions(+), 4 deletions(-) ... > +static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action, > + void *ptr) > +{ > + struct efivarfs_fs_info *sfi = container_of(nb, struct efivarfs_fs_info, > + pm_nb); > + struct path path = { .mnt = NULL, .dentry = sfi->sb->s_root, }; > + struct efivarfs_ctx ectx = { > + .ctx = { > + .actor = efivarfs_actor, > + }, > + .sb = sfi->sb, > + }; > + struct file *file; > + static bool rescan_done = true; > + > + if (action == PM_HIBERNATION_PREPARE) { > + rescan_done = false; > + return NOTIFY_OK; > + } else if (action != PM_POST_HIBERNATION) { > + return NOTIFY_DONE; > + } > + > + if (rescan_done) > + return NOTIFY_DONE; > + > + pr_info("efivarfs: resyncing variable state\n"); > + > + /* O_NOATIME is required to prevent oops on NULL mnt */ > + file = kernel_file_open(&path, O_RDONLY | O_DIRECTORY | O_NOATIME, > + current_cred()); > + if (!file) > + return NOTIFY_DONE; > + > + rescan_done = true; > + > + /* > + * First loop over the directory and verify each entry exists, > + * removing it if it doesn't > + */ > + file->f_pos = 2; /* skip . and .. */ > + do { > + ectx.dentry = NULL; > + iterate_dir(file, &ectx.ctx); > + if (ectx.dentry) { > + pr_info("efivarfs: removing variable %pd\n", > + ectx.dentry); > + simple_recursive_removal(ectx.dentry, NULL); > + dput(ectx.dentry); > + } > + } while (ectx.dentry); > + fput(file); > + > + /* > + * then loop over variables, creating them if there's no matching > + * dentry > + */ > + efivar_init(efivarfs_check_missing, sfi->sb, false); > + > + return NOTIFY_OK; > +} With the current mainline I have observed the following crash when testing suspend on one of our Tegra devices ... rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Feb 28 16:25:55 2025 [ 246.593485] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000068 [ 246.602601] Mem abort info: [ 246.602603] ESR = 0x0000000096000004 [ 246.602605] EC = 0x25: DABT (current EL), IL = 32 bits [ 246.602608] SET = 0, FnV = 0 [ 246.602610] EA = 0, S1PTW = 0 [ 246.602612] FSC = 0x04: level 0 translation fault [ 246.602615] Data abort info: [ 246.602617] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 [ 246.634959] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 246.634961] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 246.634964] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000105205000 [ 246.634967] [0000000000000068] pgd=0000000000000000, p4d=0000000000000000 [ 246.634974] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP [ 246.665796] Modules linked in: qrtr bridge stp llc usb_f_ncm usb_f_mass_storage usb_f_acm u_serial usb_f_rndis u_ether libcomposite tegra_drm btusb btrtl drm_dp_aux_bus btintel cec nvme btmtk nvme_core btbcm drm_display_helper bluetoot h snd_soc_tegra210_admaif drm_client_lib snd_soc_tegra210_dmic snd_soc_tegra186_asrc snd_soc_tegra210_mixer snd_soc_tegra_pcm snd_soc_tegra210_mvc snd_soc_tegra210_ope snd_soc_tegra210_adx snd_soc_tegra210_amx snd_soc_tegra210_sfc snd_soc _tegra210_i2s drm_kms_helper ecdh_generic tegra_se ucsi_ccg ecc snd_hda_codec_hdmi typec_ucsi snd_soc_tegra_audio_graph_card snd_soc_tegra210_ahub rfkill tegra210_adma snd_hda_tegra crypto_engine snd_soc_audio_graph_card typec snd_hda_cod ec snd_soc_simple_card_utils tegra_aconnect arm_dsu_pmu snd_soc_rt5640 snd_hda_core tegra_xudc ramoops phy_tegra194_p2u snd_soc_rl6231 at24 pcie_tegra194 tegra_bpmp_thermal host1x reed_solomon lm90 pwm_tegra ina3221 pwm_fan fuse drm backl ight dm_mod ip_tables x_tables ipv6 [ 246.756182] CPU: 9 UID: 0 PID: 1255 Comm: rtcwake Not tainted 6.14.0-rc4-g8d538b296d56 #61 [ 246.764677] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer Kit/Jetson, BIOS 00.0.0-dev-main_88214_5a0f5_a213e 02/26/2025 [ 246.776569] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 246.783718] pc : efivarfs_pm_notify+0x48/0x180 [ 246.788285] lr : blocking_notifier_call_chain_robust+0x78/0xe4 [ 246.794286] sp : ffff800085cebb60 [ 246.797684] x29: ffff800085cebb60 x28: ffff000093f1b480 x27: 0000000000000000 [ 246.805021] x26: 0000000000000004 x25: ffff8000828d3638 x24: 0000000000000003 [ 246.812355] x23: 0000000000000000 x22: 0000000000000005 x21: 0000000000000006 [ 246.819694] x20: ffff000087f698c0 x19: 0000000000000003 x18: 000000007f19bcda [ 246.827029] x17: 00000000c42545a5 x16: 000000000000001c x15: 000000001709e0a9 [ 246.834372] x14: 00000000ac6b3a37 x13: 18286bf36c021b08 x12: 00000039694cf81c [ 246.841713] x11: 00000000f1e0faad x10: 0000000000000001 x9 : 000000004ff99d57 [ 246.849046] x8 : 00000000bb51f9d6 x7 : 00000001f4d4185c x6 : 00000001f4d4185c [ 246.856382] x5 : ffff800085cebb58 x4 : ffff800080952930 x3 : ffff8000804cba44 [ 246.863713] x2 : 0000000000000000 x1 : 0000000000000003 x0 : ffff8000804cbf84 [ 246.871045] Call trace: [ 246.873550] efivarfs_pm_notify+0x48/0x180 (P) [ 246.878119] blocking_notifier_call_chain_robust+0x78/0xe4 [ 246.883753] pm_notifier_call_chain_robust+0x28/0x48 [ 246.888852] pm_suspend+0x138/0x1c8 [ 246.892438] state_store+0x8c/0xfc [ 246.895931] kobj_attr_store+0x18/0x2c [ 246.899791] sysfs_kf_write+0x44/0x54 [ 246.903553] kernfs_fop_write_iter+0x118/0x1a8 [ 246.908113] vfs_write+0x2b0/0x35c [ 246.911608] ksys_write+0x68/0xfc [ 246.915013] __arm64_sys_write+0x1c/0x28 [ 246.919038] invoke_syscall+0x48/0x110 [ 246.922897] el0_svc_common.constprop.0+0x40/0xe8 [ 246.927731] do_el0_svc+0x20/0x2c [ 246.931127] el0_svc+0x30/0xd0 [ 246.934265] el0t_64_sync_handler+0x144/0x168 [ 246.938737] el0t_64_sync+0x198/0x19c [ 246.942505] Code: f9400682 f90027ff a906ffe2 f100043f (f9403440) [ 246.948767] ---[ end trace 0000000000000000 ]--- Bisect is pointing to this commit. I had a quick look at this and the following fixes it for me ... diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c index 09fcf731e65d..1feb5d03295b 100644 --- a/fs/efivarfs/super.c +++ b/fs/efivarfs/super.c @@ -477,7 +477,7 @@ static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action, { struct efivarfs_fs_info *sfi = container_of(nb, struct efivarfs_fs_info, pm_nb); - struct path path = { .mnt = NULL, .dentry = sfi->sb->s_root, }; + struct path path = { .mnt = NULL, }; struct efivarfs_ctx ectx = { .ctx = { .actor = efivarfs_actor, @@ -487,6 +487,11 @@ static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action, struct file *file; static bool rescan_done = true; + if (!sfi || !sfi->sb) + return NOTIFY_DONE; + + path.dentry = sfi->sb->s_root; + if (action == PM_HIBERNATION_PREPARE) { rescan_done = false; return NOTIFY_OK; I am not sure if we are missing EFI variable somewhere, but I guess this is something we need to fix. Let me know what your thoughts are. Thanks! Jon
On Fri, 2025-02-28 at 16:44 +0000, Jon Hunter wrote: > Hi James, > > On 07/01/2025 21:31, James Bottomley wrote: > > Hibernation allows other OSs to boot and thus the variable state > > might > > be altered by the time the hibernation image is resumed. Resync > > the > > variable state by looping over all the dentries and update the size > > (in case of alteration) delete any which no-longer exist. Finally, > > loop over all efi variables creating any which don't have > > corresponding dentries. > > > > Signed-off-by: James Bottomley > > <James.Bottomley@HansenPartnership.com> > > --- > > fs/efivarfs/internal.h | 3 +- > > fs/efivarfs/super.c | 151 > > ++++++++++++++++++++++++++++++++++++++++- > > fs/efivarfs/vars.c | 5 +- > > 3 files changed, 155 insertions(+), 4 deletions(-) > > ... > > > +static int efivarfs_pm_notify(struct notifier_block *nb, unsigned > > long action, > > + void *ptr) > > +{ > > + struct efivarfs_fs_info *sfi = container_of(nb, struct > > efivarfs_fs_info, > > + pm_nb); > > + struct path path = { .mnt = NULL, .dentry = sfi->sb- > > >s_root, }; > > + struct efivarfs_ctx ectx = { > > + .ctx = { > > + .actor = efivarfs_actor, > > + }, > > + .sb = sfi->sb, > > + }; > > + struct file *file; > > + static bool rescan_done = true; > > + > > + if (action == PM_HIBERNATION_PREPARE) { > > + rescan_done = false; > > + return NOTIFY_OK; > > + } else if (action != PM_POST_HIBERNATION) { > > + return NOTIFY_DONE; > > + } > > + > > + if (rescan_done) > > + return NOTIFY_DONE; > > + > > + pr_info("efivarfs: resyncing variable state\n"); > > + > > + /* O_NOATIME is required to prevent oops on NULL mnt */ > > + file = kernel_file_open(&path, O_RDONLY | O_DIRECTORY | > > O_NOATIME, > > + current_cred()); > > + if (!file) > > + return NOTIFY_DONE; > > + > > + rescan_done = true; > > + > > + /* > > + * First loop over the directory and verify each entry > > exists, > > + * removing it if it doesn't > > + */ > > + file->f_pos = 2; /* skip . and .. */ > > + do { > > + ectx.dentry = NULL; > > + iterate_dir(file, &ectx.ctx); > > + if (ectx.dentry) { > > + pr_info("efivarfs: removing variable > > %pd\n", > > + ectx.dentry); > > + simple_recursive_removal(ectx.dentry, > > NULL); > > + dput(ectx.dentry); > > + } > > + } while (ectx.dentry); > > + fput(file); > > + > > + /* > > + * then loop over variables, creating them if there's no > > matching > > + * dentry > > + */ > > + efivar_init(efivarfs_check_missing, sfi->sb, false); > > + > > + return NOTIFY_OK; > > +} > > > With the current mainline I have observed the following crash when > testing suspend on one of our Tegra devices ... > > rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Feb 28 16:25:55 > 2025 > [ 246.593485] Unable to handle kernel NULL pointer dereference at > virtual address 0000000000000068 > [ 246.602601] Mem abort info: > [ 246.602603] ESR = 0x0000000096000004 > [ 246.602605] EC = 0x25: DABT (current EL), IL = 32 bits > [ 246.602608] SET = 0, FnV = 0 > [ 246.602610] EA = 0, S1PTW = 0 > [ 246.602612] FSC = 0x04: level 0 translation fault > [ 246.602615] Data abort info: > [ 246.602617] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 > [ 246.634959] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 > [ 246.634961] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [ 246.634964] user pgtable: 4k pages, 48-bit VAs, > pgdp=0000000105205000 > [ 246.634967] [0000000000000068] pgd=0000000000000000, > p4d=0000000000000000 > [ 246.634974] Internal error: Oops: 0000000096000004 [#1] PREEMPT > SMP > [ 246.665796] Modules linked in: qrtr bridge stp llc usb_f_ncm > usb_f_mass_storage usb_f_acm u_serial usb_f_rndis u_ether > libcomposite tegra_drm btusb btrtl drm_dp_aux_bus btintel cec nvme > btmtk nvme_core btbcm drm_display_helper bluetoot > h snd_soc_tegra210_admaif drm_client_lib snd_soc_tegra210_dmic > snd_soc_tegra186_asrc snd_soc_tegra210_mixer snd_soc_tegra_pcm > snd_soc_tegra210_mvc snd_soc_tegra210_ope snd_soc_tegra210_adx > snd_soc_tegra210_amx snd_soc_tegra210_sfc snd_soc > _tegra210_i2s drm_kms_helper ecdh_generic tegra_se ucsi_ccg ecc > snd_hda_codec_hdmi typec_ucsi snd_soc_tegra_audio_graph_card > snd_soc_tegra210_ahub rfkill tegra210_adma snd_hda_tegra > crypto_engine snd_soc_audio_graph_card typec snd_hda_cod > ec snd_soc_simple_card_utils tegra_aconnect arm_dsu_pmu > snd_soc_rt5640 snd_hda_core tegra_xudc ramoops phy_tegra194_p2u > snd_soc_rl6231 at24 pcie_tegra194 tegra_bpmp_thermal host1x > reed_solomon lm90 pwm_tegra ina3221 pwm_fan fuse drm backl > ight dm_mod ip_tables x_tables ipv6 > [ 246.756182] CPU: 9 UID: 0 PID: 1255 Comm: rtcwake Not tainted > 6.14.0-rc4-g8d538b296d56 #61 > [ 246.764677] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer > Kit/Jetson, BIOS 00.0.0-dev-main_88214_5a0f5_a213e 02/26/2025 > [ 246.776569] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS > BTYPE=--) > [ 246.783718] pc : efivarfs_pm_notify+0x48/0x180 > [ 246.788285] lr : blocking_notifier_call_chain_robust+0x78/0xe4 > [ 246.794286] sp : ffff800085cebb60 > [ 246.797684] x29: ffff800085cebb60 x28: ffff000093f1b480 x27: > 0000000000000000 > [ 246.805021] x26: 0000000000000004 x25: ffff8000828d3638 x24: > 0000000000000003 > [ 246.812355] x23: 0000000000000000 x22: 0000000000000005 x21: > 0000000000000006 > [ 246.819694] x20: ffff000087f698c0 x19: 0000000000000003 x18: > 000000007f19bcda > [ 246.827029] x17: 00000000c42545a5 x16: 000000000000001c x15: > 000000001709e0a9 > [ 246.834372] x14: 00000000ac6b3a37 x13: 18286bf36c021b08 x12: > 00000039694cf81c > [ 246.841713] x11: 00000000f1e0faad x10: 0000000000000001 x9 : > 000000004ff99d57 > [ 246.849046] x8 : 00000000bb51f9d6 x7 : 00000001f4d4185c x6 : > 00000001f4d4185c > [ 246.856382] x5 : ffff800085cebb58 x4 : ffff800080952930 x3 : > ffff8000804cba44 > [ 246.863713] x2 : 0000000000000000 x1 : 0000000000000003 x0 : > ffff8000804cbf84 > [ 246.871045] Call trace: > [ 246.873550] efivarfs_pm_notify+0x48/0x180 (P) > [ 246.878119] blocking_notifier_call_chain_robust+0x78/0xe4 > [ 246.883753] pm_notifier_call_chain_robust+0x28/0x48 > [ 246.888852] pm_suspend+0x138/0x1c8 > [ 246.892438] state_store+0x8c/0xfc > [ 246.895931] kobj_attr_store+0x18/0x2c > [ 246.899791] sysfs_kf_write+0x44/0x54 > [ 246.903553] kernfs_fop_write_iter+0x118/0x1a8 > [ 246.908113] vfs_write+0x2b0/0x35c > [ 246.911608] ksys_write+0x68/0xfc > [ 246.915013] __arm64_sys_write+0x1c/0x28 > [ 246.919038] invoke_syscall+0x48/0x110 > [ 246.922897] el0_svc_common.constprop.0+0x40/0xe8 > [ 246.927731] do_el0_svc+0x20/0x2c > [ 246.931127] el0_svc+0x30/0xd0 > [ 246.934265] el0t_64_sync_handler+0x144/0x168 > [ 246.938737] el0t_64_sync+0x198/0x19c > [ 246.942505] Code: f9400682 f90027ff a906ffe2 f100043f (f9403440) > [ 246.948767] ---[ end trace 0000000000000000 ]--- > > > Bisect is pointing to this commit. I had a quick look at this and the > following fixes it for me ... Yes, this occurs because the notifier can be triggered before the superblock information is filled. Ard fixed this: https://web.git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=urgent&id=cb6ae457bc6af58c84a7854df5e7e32ba1c6a715 Regards, James
On 28/02/2025 16:49, James Bottomley wrote: > On Fri, 2025-02-28 at 16:44 +0000, Jon Hunter wrote: >> Hi James, >> >> On 07/01/2025 21:31, James Bottomley wrote: >>> Hibernation allows other OSs to boot and thus the variable state >>> might >>> be altered by the time the hibernation image is resumed. Resync >>> the >>> variable state by looping over all the dentries and update the size >>> (in case of alteration) delete any which no-longer exist. Finally, >>> loop over all efi variables creating any which don't have >>> corresponding dentries. >>> >>> Signed-off-by: James Bottomley >>> <James.Bottomley@HansenPartnership.com> >>> --- >>> fs/efivarfs/internal.h | 3 +- >>> fs/efivarfs/super.c | 151 >>> ++++++++++++++++++++++++++++++++++++++++- >>> fs/efivarfs/vars.c | 5 +- >>> 3 files changed, 155 insertions(+), 4 deletions(-) >> >> ... >> >>> +static int efivarfs_pm_notify(struct notifier_block *nb, unsigned >>> long action, >>> + void *ptr) >>> +{ >>> + struct efivarfs_fs_info *sfi = container_of(nb, struct >>> efivarfs_fs_info, >>> + pm_nb); >>> + struct path path = { .mnt = NULL, .dentry = sfi->sb- >>>> s_root, }; >>> + struct efivarfs_ctx ectx = { >>> + .ctx = { >>> + .actor = efivarfs_actor, >>> + }, >>> + .sb = sfi->sb, >>> + }; >>> + struct file *file; >>> + static bool rescan_done = true; >>> + >>> + if (action == PM_HIBERNATION_PREPARE) { >>> + rescan_done = false; >>> + return NOTIFY_OK; >>> + } else if (action != PM_POST_HIBERNATION) { >>> + return NOTIFY_DONE; >>> + } >>> + >>> + if (rescan_done) >>> + return NOTIFY_DONE; >>> + >>> + pr_info("efivarfs: resyncing variable state\n"); >>> + >>> + /* O_NOATIME is required to prevent oops on NULL mnt */ >>> + file = kernel_file_open(&path, O_RDONLY | O_DIRECTORY | >>> O_NOATIME, >>> + current_cred()); >>> + if (!file) >>> + return NOTIFY_DONE; >>> + >>> + rescan_done = true; >>> + >>> + /* >>> + * First loop over the directory and verify each entry >>> exists, >>> + * removing it if it doesn't >>> + */ >>> + file->f_pos = 2; /* skip . and .. */ >>> + do { >>> + ectx.dentry = NULL; >>> + iterate_dir(file, &ectx.ctx); >>> + if (ectx.dentry) { >>> + pr_info("efivarfs: removing variable >>> %pd\n", >>> + ectx.dentry); >>> + simple_recursive_removal(ectx.dentry, >>> NULL); >>> + dput(ectx.dentry); >>> + } >>> + } while (ectx.dentry); >>> + fput(file); >>> + >>> + /* >>> + * then loop over variables, creating them if there's no >>> matching >>> + * dentry >>> + */ >>> + efivar_init(efivarfs_check_missing, sfi->sb, false); >>> + >>> + return NOTIFY_OK; >>> +} >> >> >> With the current mainline I have observed the following crash when >> testing suspend on one of our Tegra devices ... >> >> rtcwake: wakeup from "mem" using /dev/rtc0 at Fri Feb 28 16:25:55 >> 2025 >> [ 246.593485] Unable to handle kernel NULL pointer dereference at >> virtual address 0000000000000068 >> [ 246.602601] Mem abort info: >> [ 246.602603] ESR = 0x0000000096000004 >> [ 246.602605] EC = 0x25: DABT (current EL), IL = 32 bits >> [ 246.602608] SET = 0, FnV = 0 >> [ 246.602610] EA = 0, S1PTW = 0 >> [ 246.602612] FSC = 0x04: level 0 translation fault >> [ 246.602615] Data abort info: >> [ 246.602617] ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000 >> [ 246.634959] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 >> [ 246.634961] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 >> [ 246.634964] user pgtable: 4k pages, 48-bit VAs, >> pgdp=0000000105205000 >> [ 246.634967] [0000000000000068] pgd=0000000000000000, >> p4d=0000000000000000 >> [ 246.634974] Internal error: Oops: 0000000096000004 [#1] PREEMPT >> SMP >> [ 246.665796] Modules linked in: qrtr bridge stp llc usb_f_ncm >> usb_f_mass_storage usb_f_acm u_serial usb_f_rndis u_ether >> libcomposite tegra_drm btusb btrtl drm_dp_aux_bus btintel cec nvme >> btmtk nvme_core btbcm drm_display_helper bluetoot >> h snd_soc_tegra210_admaif drm_client_lib snd_soc_tegra210_dmic >> snd_soc_tegra186_asrc snd_soc_tegra210_mixer snd_soc_tegra_pcm >> snd_soc_tegra210_mvc snd_soc_tegra210_ope snd_soc_tegra210_adx >> snd_soc_tegra210_amx snd_soc_tegra210_sfc snd_soc >> _tegra210_i2s drm_kms_helper ecdh_generic tegra_se ucsi_ccg ecc >> snd_hda_codec_hdmi typec_ucsi snd_soc_tegra_audio_graph_card >> snd_soc_tegra210_ahub rfkill tegra210_adma snd_hda_tegra >> crypto_engine snd_soc_audio_graph_card typec snd_hda_cod >> ec snd_soc_simple_card_utils tegra_aconnect arm_dsu_pmu >> snd_soc_rt5640 snd_hda_core tegra_xudc ramoops phy_tegra194_p2u >> snd_soc_rl6231 at24 pcie_tegra194 tegra_bpmp_thermal host1x >> reed_solomon lm90 pwm_tegra ina3221 pwm_fan fuse drm backl >> ight dm_mod ip_tables x_tables ipv6 >> [ 246.756182] CPU: 9 UID: 0 PID: 1255 Comm: rtcwake Not tainted >> 6.14.0-rc4-g8d538b296d56 #61 >> [ 246.764677] Hardware name: NVIDIA NVIDIA Jetson AGX Orin Developer >> Kit/Jetson, BIOS 00.0.0-dev-main_88214_5a0f5_a213e 02/26/2025 >> [ 246.776569] pstate: 20400009 (nzCv daif +PAN -UAO -TCO -DIT -SSBS >> BTYPE=--) >> [ 246.783718] pc : efivarfs_pm_notify+0x48/0x180 >> [ 246.788285] lr : blocking_notifier_call_chain_robust+0x78/0xe4 >> [ 246.794286] sp : ffff800085cebb60 >> [ 246.797684] x29: ffff800085cebb60 x28: ffff000093f1b480 x27: >> 0000000000000000 >> [ 246.805021] x26: 0000000000000004 x25: ffff8000828d3638 x24: >> 0000000000000003 >> [ 246.812355] x23: 0000000000000000 x22: 0000000000000005 x21: >> 0000000000000006 >> [ 246.819694] x20: ffff000087f698c0 x19: 0000000000000003 x18: >> 000000007f19bcda >> [ 246.827029] x17: 00000000c42545a5 x16: 000000000000001c x15: >> 000000001709e0a9 >> [ 246.834372] x14: 00000000ac6b3a37 x13: 18286bf36c021b08 x12: >> 00000039694cf81c >> [ 246.841713] x11: 00000000f1e0faad x10: 0000000000000001 x9 : >> 000000004ff99d57 >> [ 246.849046] x8 : 00000000bb51f9d6 x7 : 00000001f4d4185c x6 : >> 00000001f4d4185c >> [ 246.856382] x5 : ffff800085cebb58 x4 : ffff800080952930 x3 : >> ffff8000804cba44 >> [ 246.863713] x2 : 0000000000000000 x1 : 0000000000000003 x0 : >> ffff8000804cbf84 >> [ 246.871045] Call trace: >> [ 246.873550] efivarfs_pm_notify+0x48/0x180 (P) >> [ 246.878119] blocking_notifier_call_chain_robust+0x78/0xe4 >> [ 246.883753] pm_notifier_call_chain_robust+0x28/0x48 >> [ 246.888852] pm_suspend+0x138/0x1c8 >> [ 246.892438] state_store+0x8c/0xfc >> [ 246.895931] kobj_attr_store+0x18/0x2c >> [ 246.899791] sysfs_kf_write+0x44/0x54 >> [ 246.903553] kernfs_fop_write_iter+0x118/0x1a8 >> [ 246.908113] vfs_write+0x2b0/0x35c >> [ 246.911608] ksys_write+0x68/0xfc >> [ 246.915013] __arm64_sys_write+0x1c/0x28 >> [ 246.919038] invoke_syscall+0x48/0x110 >> [ 246.922897] el0_svc_common.constprop.0+0x40/0xe8 >> [ 246.927731] do_el0_svc+0x20/0x2c >> [ 246.931127] el0_svc+0x30/0xd0 >> [ 246.934265] el0t_64_sync_handler+0x144/0x168 >> [ 246.938737] el0t_64_sync+0x198/0x19c >> [ 246.942505] Code: f9400682 f90027ff a906ffe2 f100043f (f9403440) >> [ 246.948767] ---[ end trace 0000000000000000 ]--- >> >> >> Bisect is pointing to this commit. I had a quick look at this and the >> following fixes it for me ... > > Yes, this occurs because the notifier can be triggered before the > superblock information is filled. Ard fixed this: > > https://web.git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/commit/?h=urgent&id=cb6ae457bc6af58c84a7854df5e7e32ba1c6a715 Thanks for the quick response. I can confirm that this fixes it for me. Feel free to add my ... Tested-by: Jon Hunter <jonathanh@nvidia.com> Cheers Jon
diff --git a/fs/efivarfs/internal.h b/fs/efivarfs/internal.h index 32b83f644798..48ab75d69b26 100644 --- a/fs/efivarfs/internal.h +++ b/fs/efivarfs/internal.h @@ -17,6 +17,7 @@ struct efivarfs_fs_info { struct efivarfs_mount_opts mount_opts; struct super_block *sb; struct notifier_block nb; + struct notifier_block pm_nb; }; struct efi_variable { @@ -30,7 +31,7 @@ struct efivar_entry { }; int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *), - void *data); + void *data, bool duplicate_check); int efivar_entry_delete(struct efivar_entry *entry); diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c index 2523030caeda..961264f628dc 100644 --- a/fs/efivarfs/super.c +++ b/fs/efivarfs/super.c @@ -13,6 +13,7 @@ #include <linux/pagemap.h> #include <linux/ucs2_string.h> #include <linux/slab.h> +#include <linux/suspend.h> #include <linux/magic.h> #include <linux/statfs.h> #include <linux/notifier.h> @@ -356,7 +357,7 @@ static int efivarfs_fill_super(struct super_block *sb, struct fs_context *fc) if (err) return err; - return efivar_init(efivarfs_callback, sb); + return efivar_init(efivarfs_callback, sb, true); } static int efivarfs_get_tree(struct fs_context *fc) @@ -380,6 +381,148 @@ static const struct fs_context_operations efivarfs_context_ops = { .reconfigure = efivarfs_reconfigure, }; +struct efivarfs_ctx { + struct dir_context ctx; + struct super_block *sb; + struct dentry *dentry; +}; + +static bool efivarfs_actor(struct dir_context *ctx, const char *name, int len, + loff_t offset, u64 ino, unsigned mode) +{ + unsigned long size; + struct efivarfs_ctx *ectx = container_of(ctx, struct efivarfs_ctx, ctx); + struct qstr qstr = { .name = name, .len = len }; + struct dentry *dentry = d_hash_and_lookup(ectx->sb->s_root, &qstr); + struct inode *inode; + struct efivar_entry *entry; + int err; + + if (IS_ERR_OR_NULL(dentry)) + return true; + + inode = d_inode(dentry); + entry = inode->i_private; + + err = efivar_entry_size(entry, &size); + size += sizeof(__u32); /* attributes */ + if (err) + size = 0; + + inode_lock(inode); + i_size_write(inode, size); + inode_unlock(inode); + + if (!size) { + ectx->dentry = dentry; + return false; + } + + dput(dentry); + + return true; +} + +static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor, + unsigned long name_size, void *data) +{ + char *name; + struct super_block *sb = data; + struct dentry *dentry; + struct qstr qstr; + int err; + + if (guid_equal(&vendor, &LINUX_EFI_RANDOM_SEED_TABLE_GUID)) + return 0; + + name = efivar_get_utf8name(name16, &vendor); + if (!name) + return -ENOMEM; + + qstr.name = name; + qstr.len = strlen(name); + dentry = d_hash_and_lookup(sb->s_root, &qstr); + if (IS_ERR(dentry)) { + err = PTR_ERR(dentry); + goto out; + } + + if (!dentry) { + /* found missing entry */ + pr_info("efivarfs: creating variable %s\n", name); + return efivarfs_create_dentry(sb, name16, name_size, &vendor, name); + } + + dput(dentry); + err = 0; + + out: + kfree(name); + + return err; +} + +static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action, + void *ptr) +{ + struct efivarfs_fs_info *sfi = container_of(nb, struct efivarfs_fs_info, + pm_nb); + struct path path = { .mnt = NULL, .dentry = sfi->sb->s_root, }; + struct efivarfs_ctx ectx = { + .ctx = { + .actor = efivarfs_actor, + }, + .sb = sfi->sb, + }; + struct file *file; + static bool rescan_done = true; + + if (action == PM_HIBERNATION_PREPARE) { + rescan_done = false; + return NOTIFY_OK; + } else if (action != PM_POST_HIBERNATION) { + return NOTIFY_DONE; + } + + if (rescan_done) + return NOTIFY_DONE; + + pr_info("efivarfs: resyncing variable state\n"); + + /* O_NOATIME is required to prevent oops on NULL mnt */ + file = kernel_file_open(&path, O_RDONLY | O_DIRECTORY | O_NOATIME, + current_cred()); + if (!file) + return NOTIFY_DONE; + + rescan_done = true; + + /* + * First loop over the directory and verify each entry exists, + * removing it if it doesn't + */ + file->f_pos = 2; /* skip . and .. */ + do { + ectx.dentry = NULL; + iterate_dir(file, &ectx.ctx); + if (ectx.dentry) { + pr_info("efivarfs: removing variable %pd\n", + ectx.dentry); + simple_recursive_removal(ectx.dentry, NULL); + dput(ectx.dentry); + } + } while (ectx.dentry); + fput(file); + + /* + * then loop over variables, creating them if there's no matching + * dentry + */ + efivar_init(efivarfs_check_missing, sfi->sb, false); + + return NOTIFY_OK; +} + static int efivarfs_init_fs_context(struct fs_context *fc) { struct efivarfs_fs_info *sfi; @@ -396,6 +539,11 @@ static int efivarfs_init_fs_context(struct fs_context *fc) fc->s_fs_info = sfi; fc->ops = &efivarfs_context_ops; + + sfi->pm_nb.notifier_call = efivarfs_pm_notify; + sfi->pm_nb.priority = 0; + register_pm_notifier(&sfi->pm_nb); + return 0; } @@ -405,6 +553,7 @@ static void efivarfs_kill_sb(struct super_block *sb) blocking_notifier_chain_unregister(&efivar_ops_nh, &sfi->nb); kill_litter_super(sb); + unregister_pm_notifier(&sfi->pm_nb); kfree(sfi); } diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c index d0beecbf9441..d720d780648b 100644 --- a/fs/efivarfs/vars.c +++ b/fs/efivarfs/vars.c @@ -371,7 +371,7 @@ static void dup_variable_bug(efi_char16_t *str16, efi_guid_t *vendor_guid, * Returns 0 on success, or a kernel error code on failure. */ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *), - void *data) + void *data, bool duplicate_check) { unsigned long variable_name_size = 512; efi_char16_t *variable_name; @@ -415,7 +415,8 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *), * we'll ever see a different variable name, * and may end up looping here forever. */ - if (efivarfs_variable_is_present(variable_name, + if (duplicate_check && + efivarfs_variable_is_present(variable_name, &vendor_guid, data)) { dup_variable_bug(variable_name, &vendor_guid, variable_name_size);
Hibernation allows other OSs to boot and thus the variable state might be altered by the time the hibernation image is resumed. Resync the variable state by looping over all the dentries and update the size (in case of alteration) delete any which no-longer exist. Finally, loop over all efi variables creating any which don't have corresponding dentries. Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> --- fs/efivarfs/internal.h | 3 +- fs/efivarfs/super.c | 151 ++++++++++++++++++++++++++++++++++++++++- fs/efivarfs/vars.c | 5 +- 3 files changed, 155 insertions(+), 4 deletions(-)