Skip to content

Commit 544f629

Browse files
Sylvain Chouleurmfleming
Sylvain Chouleur
authored and
mfleming
committed
efi: Use a file local lock for efivars
This patch replaces the spinlock in the efivars struct with a single lock for the whole vars.c file. The goal of this lock is to protect concurrent calls to efi variable services, registering and unregistering. This allows us to register new efivars operations without having in-progress call. Signed-off-by: Sylvain Chouleur <sylvain.chouleur@intel.com> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Cc: Leif Lindholm <leif.lindholm@linaro.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Sylvain Chouleur <sylvain.chouleur@gmail.com> Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
1 parent a720c7e commit 544f629

File tree

2 files changed

+47
-42
lines changed

2 files changed

+47
-42
lines changed

‎drivers/firmware/efi/vars.c

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@
3737
/* Private pointer to registered efivars */
3838
static struct efivars *__efivars;
3939

40+
/*
41+
* efivars_lock protects three things:
42+
* 1) efivarfs_list and efivars_sysfs_list
43+
* 2) ->ops calls
44+
* 3) (un)registration of __efivars
45+
*/
46+
static DEFINE_SPINLOCK(efivars_lock);
47+
4048
static bool efivar_wq_enabled = true;
4149
DECLARE_WORK(efivar_work, NULL);
4250
EXPORT_SYMBOL_GPL(efivar_work);
@@ -434,7 +442,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
434442
return -ENOMEM;
435443
}
436444

437-
spin_lock_irq(&__efivars->lock);
445+
spin_lock_irq(&efivars_lock);
438446

439447
/*
440448
* Per EFI spec, the maximum storage allocated for both
@@ -450,7 +458,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
450458
switch (status) {
451459
case EFI_SUCCESS:
452460
if (duplicates)
453-
spin_unlock_irq(&__efivars->lock);
461+
spin_unlock_irq(&efivars_lock);
454462

455463
variable_name_size = var_name_strnsize(variable_name,
456464
variable_name_size);
@@ -477,7 +485,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
477485
}
478486

479487
if (duplicates)
480-
spin_lock_irq(&__efivars->lock);
488+
spin_lock_irq(&efivars_lock);
481489

482490
break;
483491
case EFI_NOT_FOUND:
@@ -491,7 +499,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t, unsigned long, void *),
491499

492500
} while (status != EFI_NOT_FOUND);
493501

494-
spin_unlock_irq(&__efivars->lock);
502+
spin_unlock_irq(&efivars_lock);
495503

496504
kfree(variable_name);
497505

@@ -506,9 +514,9 @@ EXPORT_SYMBOL_GPL(efivar_init);
506514
*/
507515
void efivar_entry_add(struct efivar_entry *entry, struct list_head *head)
508516
{
509-
spin_lock_irq(&__efivars->lock);
517+
spin_lock_irq(&efivars_lock);
510518
list_add(&entry->list, head);
511-
spin_unlock_irq(&__efivars->lock);
519+
spin_unlock_irq(&efivars_lock);
512520
}
513521
EXPORT_SYMBOL_GPL(efivar_entry_add);
514522

@@ -518,9 +526,9 @@ EXPORT_SYMBOL_GPL(efivar_entry_add);
518526
*/
519527
void efivar_entry_remove(struct efivar_entry *entry)
520528
{
521-
spin_lock_irq(&__efivars->lock);
529+
spin_lock_irq(&efivars_lock);
522530
list_del(&entry->list);
523-
spin_unlock_irq(&__efivars->lock);
531+
spin_unlock_irq(&efivars_lock);
524532
}
525533
EXPORT_SYMBOL_GPL(efivar_entry_remove);
526534

@@ -537,10 +545,10 @@ EXPORT_SYMBOL_GPL(efivar_entry_remove);
537545
*/
538546
static void efivar_entry_list_del_unlock(struct efivar_entry *entry)
539547
{
540-
lockdep_assert_held(&__efivars->lock);
548+
lockdep_assert_held(&efivars_lock);
541549

542550
list_del(&entry->list);
543-
spin_unlock_irq(&__efivars->lock);
551+
spin_unlock_irq(&efivars_lock);
544552
}
545553

546554
/**
@@ -563,7 +571,7 @@ int __efivar_entry_delete(struct efivar_entry *entry)
563571
const struct efivar_operations *ops = __efivars->ops;
564572
efi_status_t status;
565573

566-
lockdep_assert_held(&__efivars->lock);
574+
lockdep_assert_held(&efivars_lock);
567575

568576
status = ops->set_variable(entry->var.VariableName,
569577
&entry->var.VendorGuid,
@@ -589,12 +597,12 @@ int efivar_entry_delete(struct efivar_entry *entry)
589597
const struct efivar_operations *ops = __efivars->ops;
590598
efi_status_t status;
591599

592-
spin_lock_irq(&__efivars->lock);
600+
spin_lock_irq(&efivars_lock);
593601
status = ops->set_variable(entry->var.VariableName,
594602
&entry->var.VendorGuid,
595603
0, 0, NULL);
596604
if (!(status == EFI_SUCCESS || status == EFI_NOT_FOUND)) {
597-
spin_unlock_irq(&__efivars->lock);
605+
spin_unlock_irq(&efivars_lock);
598606
return efi_status_to_err(status);
599607
}
600608

@@ -632,10 +640,10 @@ int efivar_entry_set(struct efivar_entry *entry, u32 attributes,
632640
efi_char16_t *name = entry->var.VariableName;
633641
efi_guid_t vendor = entry->var.VendorGuid;
634642

635-
spin_lock_irq(&__efivars->lock);
643+
spin_lock_irq(&efivars_lock);
636644

637645
if (head && efivar_entry_find(name, vendor, head, false)) {
638-
spin_unlock_irq(&__efivars->lock);
646+
spin_unlock_irq(&efivars_lock);
639647
return -EEXIST;
640648
}
641649

@@ -644,7 +652,7 @@ int efivar_entry_set(struct efivar_entry *entry, u32 attributes,
644652
status = ops->set_variable(name, &vendor,
645653
attributes, size, data);
646654

647-
spin_unlock_irq(&__efivars->lock);
655+
spin_unlock_irq(&efivars_lock);
648656

649657
return efi_status_to_err(status);
650658

@@ -658,7 +666,7 @@ EXPORT_SYMBOL_GPL(efivar_entry_set);
658666
* from crash/panic handlers.
659667
*
660668
* Crucially, this function will not block if it cannot acquire
661-
* __efivars->lock. Instead, it returns -EBUSY.
669+
* efivars_lock. Instead, it returns -EBUSY.
662670
*/
663671
static int
664672
efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor,
@@ -668,20 +676,20 @@ efivar_entry_set_nonblocking(efi_char16_t *name, efi_guid_t vendor,
668676
unsigned long flags;
669677
efi_status_t status;
670678

671-
if (!spin_trylock_irqsave(&__efivars->lock, flags))
679+
if (!spin_trylock_irqsave(&efivars_lock, flags))
672680
return -EBUSY;
673681

674682
status = check_var_size_nonblocking(attributes,
675683
size + ucs2_strsize(name, 1024));
676684
if (status != EFI_SUCCESS) {
677-
spin_unlock_irqrestore(&__efivars->lock, flags);
685+
spin_unlock_irqrestore(&efivars_lock, flags);
678686
return -ENOSPC;
679687
}
680688

681689
status = ops->set_variable_nonblocking(name, &vendor, attributes,
682690
size, data);
683691

684-
spin_unlock_irqrestore(&__efivars->lock, flags);
692+
spin_unlock_irqrestore(&efivars_lock, flags);
685693
return efi_status_to_err(status);
686694
}
687695

@@ -727,21 +735,21 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes,
727735
size, data);
728736

729737
if (!block) {
730-
if (!spin_trylock_irqsave(&__efivars->lock, flags))
738+
if (!spin_trylock_irqsave(&efivars_lock, flags))
731739
return -EBUSY;
732740
} else {
733-
spin_lock_irqsave(&__efivars->lock, flags);
741+
spin_lock_irqsave(&efivars_lock, flags);
734742
}
735743

736744
status = check_var_size(attributes, size + ucs2_strsize(name, 1024));
737745
if (status != EFI_SUCCESS) {
738-
spin_unlock_irqrestore(&__efivars->lock, flags);
746+
spin_unlock_irqrestore(&efivars_lock, flags);
739747
return -ENOSPC;
740748
}
741749

742750
status = ops->set_variable(name, &vendor, attributes, size, data);
743751

744-
spin_unlock_irqrestore(&__efivars->lock, flags);
752+
spin_unlock_irqrestore(&efivars_lock, flags);
745753

746754
return efi_status_to_err(status);
747755
}
@@ -771,7 +779,7 @@ struct efivar_entry *efivar_entry_find(efi_char16_t *name, efi_guid_t guid,
771779
int strsize1, strsize2;
772780
bool found = false;
773781

774-
lockdep_assert_held(&__efivars->lock);
782+
lockdep_assert_held(&efivars_lock);
775783

776784
list_for_each_entry_safe(entry, n, head, list) {
777785
strsize1 = ucs2_strsize(name, 1024);
@@ -814,10 +822,10 @@ int efivar_entry_size(struct efivar_entry *entry, unsigned long *size)
814822

815823
*size = 0;
816824

817-
spin_lock_irq(&__efivars->lock);
825+
spin_lock_irq(&efivars_lock);
818826
status = ops->get_variable(entry->var.VariableName,
819827
&entry->var.VendorGuid, NULL, size, NULL);
820-
spin_unlock_irq(&__efivars->lock);
828+
spin_unlock_irq(&efivars_lock);
821829

822830
if (status != EFI_BUFFER_TOO_SMALL)
823831
return efi_status_to_err(status);
@@ -843,7 +851,7 @@ int __efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
843851
const struct efivar_operations *ops = __efivars->ops;
844852
efi_status_t status;
845853

846-
lockdep_assert_held(&__efivars->lock);
854+
lockdep_assert_held(&efivars_lock);
847855

848856
status = ops->get_variable(entry->var.VariableName,
849857
&entry->var.VendorGuid,
@@ -866,11 +874,11 @@ int efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
866874
const struct efivar_operations *ops = __efivars->ops;
867875
efi_status_t status;
868876

869-
spin_lock_irq(&__efivars->lock);
877+
spin_lock_irq(&efivars_lock);
870878
status = ops->get_variable(entry->var.VariableName,
871879
&entry->var.VendorGuid,
872880
attributes, size, data);
873-
spin_unlock_irq(&__efivars->lock);
881+
spin_unlock_irq(&efivars_lock);
874882

875883
return efi_status_to_err(status);
876884
}
@@ -917,7 +925,7 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
917925
* set_variable call, and removal of the variable from the efivars
918926
* list (in the case of an authenticated delete).
919927
*/
920-
spin_lock_irq(&__efivars->lock);
928+
spin_lock_irq(&efivars_lock);
921929

922930
/*
923931
* Ensure that the available space hasn't shrunk below the safe level
@@ -957,15 +965,15 @@ int efivar_entry_set_get_size(struct efivar_entry *entry, u32 attributes,
957965
if (status == EFI_NOT_FOUND)
958966
efivar_entry_list_del_unlock(entry);
959967
else
960-
spin_unlock_irq(&__efivars->lock);
968+
spin_unlock_irq(&efivars_lock);
961969

962970
if (status && status != EFI_BUFFER_TOO_SMALL)
963971
return efi_status_to_err(status);
964972

965973
return 0;
966974

967975
out:
968-
spin_unlock_irq(&__efivars->lock);
976+
spin_unlock_irq(&efivars_lock);
969977
return err;
970978

971979
}
@@ -980,7 +988,7 @@ EXPORT_SYMBOL_GPL(efivar_entry_set_get_size);
980988
*/
981989
void efivar_entry_iter_begin(void)
982990
{
983-
spin_lock_irq(&__efivars->lock);
991+
spin_lock_irq(&efivars_lock);
984992
}
985993
EXPORT_SYMBOL_GPL(efivar_entry_iter_begin);
986994

@@ -991,7 +999,7 @@ EXPORT_SYMBOL_GPL(efivar_entry_iter_begin);
991999
*/
9921000
void efivar_entry_iter_end(void)
9931001
{
994-
spin_unlock_irq(&__efivars->lock);
1002+
spin_unlock_irq(&efivars_lock);
9951003
}
9961004
EXPORT_SYMBOL_GPL(efivar_entry_iter_end);
9971005

@@ -1112,11 +1120,12 @@ int efivars_register(struct efivars *efivars,
11121120
const struct efivar_operations *ops,
11131121
struct kobject *kobject)
11141122
{
1115-
spin_lock_init(&efivars->lock);
1123+
spin_lock_irq(&efivars_lock);
11161124
efivars->ops = ops;
11171125
efivars->kobject = kobject;
11181126

11191127
__efivars = efivars;
1128+
spin_unlock_irq(&efivars_lock);
11201129

11211130
return 0;
11221131
}
@@ -1133,6 +1142,7 @@ int efivars_unregister(struct efivars *efivars)
11331142
{
11341143
int rv;
11351144

1145+
spin_lock_irq(&efivars_lock);
11361146
if (!__efivars) {
11371147
printk(KERN_ERR "efivars not registered\n");
11381148
rv = -EINVAL;
@@ -1148,6 +1158,7 @@ int efivars_unregister(struct efivars *efivars)
11481158

11491159
rv = 0;
11501160
out:
1161+
spin_unlock_irq(&efivars_lock);
11511162
return rv;
11521163
}
11531164
EXPORT_SYMBOL_GPL(efivars_unregister);

‎include/linux/efi.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1157,12 +1157,6 @@ struct efivar_operations {
11571157
};
11581158

11591159
struct efivars {
1160-
/*
1161-
* ->lock protects two things:
1162-
* 1) efivarfs_list and efivars_sysfs_list
1163-
* 2) ->ops calls
1164-
*/
1165-
spinlock_t lock;
11661160
struct kset *kset;
11671161
struct kobject *kobject;
11681162
const struct efivar_operations *ops;

0 commit comments

Comments
 (0)