Skip to content

Commit 11092db

Browse files
James Bottomleyardbiesheuvel
authored andcommitted
efivarfs: fix NULL dereference on resume
LSMs often inspect the path.mnt of files in the security hooks, and this causes a NULL deref in efivarfs_pm_notify() because the path is constructed with a NULL path.mnt. Fix by obtaining from vfs_kern_mount() instead, and being very careful to ensure that deactivate_super() (potentially triggered by a racing userspace umount) is not called directly from the notifier, because it would deadlock when efivarfs_kill_sb() tried to unregister the notifier chain. [ Al notes: Umm... That's probably safe, but not as a long-term solution - it's too intimately dependent upon fs/super.c internals. The reasons why you can't run into ->s_umount deadlock here are non-trivial... ] Cc: Al Viro <viro@zeniv.linux.org.uk> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Link: https://lore.kernel.org/r/e54e6a2f-1178-4980-b771-4d9bafc2aa47@tnxip.de Link: https://lore.kernel.org/r/3e998bf87638a442cbc6864cdcd3d8d9e08ce3e3.camel@HansenPartnership.com Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
1 parent dec1277 commit 11092db

File tree

1 file changed

+48
-2
lines changed

1 file changed

+48
-2
lines changed

fs/efivarfs/super.c

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,19 +474,33 @@ static int efivarfs_check_missing(efi_char16_t *name16, efi_guid_t vendor,
474474
return err;
475475
}
476476

477+
static void efivarfs_deactivate_super_work(struct work_struct *work)
478+
{
479+
struct super_block *s = container_of(work, struct super_block,
480+
destroy_work);
481+
/*
482+
* note: here s->destroy_work is free for reuse (which
483+
* will happen in deactivate_super)
484+
*/
485+
deactivate_super(s);
486+
}
487+
488+
static struct file_system_type efivarfs_type;
489+
477490
static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action,
478491
void *ptr)
479492
{
480493
struct efivarfs_fs_info *sfi = container_of(nb, struct efivarfs_fs_info,
481494
pm_nb);
482-
struct path path = { .mnt = NULL, .dentry = sfi->sb->s_root, };
495+
struct path path;
483496
struct efivarfs_ctx ectx = {
484497
.ctx = {
485498
.actor = efivarfs_actor,
486499
},
487500
.sb = sfi->sb,
488501
};
489502
struct file *file;
503+
struct super_block *s = sfi->sb;
490504
static bool rescan_done = true;
491505

492506
if (action == PM_HIBERNATION_PREPARE) {
@@ -499,11 +513,43 @@ static int efivarfs_pm_notify(struct notifier_block *nb, unsigned long action,
499513
if (rescan_done)
500514
return NOTIFY_DONE;
501515

516+
/* ensure single superblock is alive and pin it */
517+
if (!atomic_inc_not_zero(&s->s_active))
518+
return NOTIFY_DONE;
519+
502520
pr_info("efivarfs: resyncing variable state\n");
503521

504-
/* O_NOATIME is required to prevent oops on NULL mnt */
522+
path.dentry = sfi->sb->s_root;
523+
524+
/*
525+
* do not add SB_KERNMOUNT which a single superblock could
526+
* expose to userspace and which also causes MNT_INTERNAL, see
527+
* below
528+
*/
529+
path.mnt = vfs_kern_mount(&efivarfs_type, 0,
530+
efivarfs_type.name, NULL);
531+
if (IS_ERR(path.mnt)) {
532+
pr_err("efivarfs: internal mount failed\n");
533+
/*
534+
* We may be the last pinner of the superblock but
535+
* calling efivarfs_kill_sb from within the notifier
536+
* here would deadlock trying to unregister it
537+
*/
538+
INIT_WORK(&s->destroy_work, efivarfs_deactivate_super_work);
539+
schedule_work(&s->destroy_work);
540+
return PTR_ERR(path.mnt);
541+
}
542+
543+
/* path.mnt now has pin on superblock, so this must be above one */
544+
atomic_dec(&s->s_active);
545+
505546
file = kernel_file_open(&path, O_RDONLY | O_DIRECTORY | O_NOATIME,
506547
current_cred());
548+
/*
549+
* safe even if last put because no MNT_INTERNAL means this
550+
* will do delayed deactivate_super and not deadlock
551+
*/
552+
mntput(path.mnt);
507553
if (IS_ERR(file))
508554
return NOTIFY_DONE;
509555

0 commit comments

Comments
 (0)