| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 3 Aug 2011 22:04:14 +0100
From: Matt Fleming <matt@...sole-pimps.org>
To: linux-kernel@...r.kernel.org
Cc: Matt Fleming <matt.fleming@...ux.intel.com>,
Jan Beulich <JBeulich@...ell.com>,
Matthew Garrett <mjg@...hat.com>,
"H. Peter Anvin" <hpa@...ux.intel.com>,
Tony Luck <tony.luck@...el.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Alessandro Zummo <a.zummo@...ertech.it>
Subject: [PATCH] x86, efi: Don't recursively acquire rtc_lock
From: Matt Fleming <matt.fleming@...ux.intel.com>
A deadlock was introduced on x86 in commit ef68c8f87ed1 ("x86:
Serialize EFI time accesses on rtc_lock") because efi_get_time() and
friends can be called with rtc_lock already held by
read_persistent_time(), e.g.
timekeeping_init()
read_persistent_clock() <-- acquire rtc_lock
efi_get_time()
phys_efi_get_time() <-- acquire rtc_lock <DEADLOCK>
Move the locking up into the caller of efi.get_time() and provide some
wrappers for use in other parts of the kernel instead of calling
efi.get_time(), etc directly. This way we can hide the rtc_lock dance
inside of arch/x86.
Cc: Jan Beulich <JBeulich@...ell.com>
Cc: Matthew Garrett <mjg@...hat.com>
Cc: "H. Peter Anvin" <hpa@...ux.intel.com>
Cc: Tony Luck <tony.luck@...el.com>
Cc: Fenghua Yu <fenghua.yu@...el.com>
Cc: Alessandro Zummo <a.zummo@...ertech.it>
Signed-off-by: Matt Fleming <matt.fleming@...ux.intel.com>
---
arch/ia64/kernel/efi.c | 10 ++++++++
arch/x86/platform/efi/efi.c | 49 ++++++++++++++++++++++++++----------------
drivers/char/efirtc.c | 6 ++--
drivers/rtc/rtc-efi.c | 8 +++---
include/linux/efi.h | 4 ++-
5 files changed, 50 insertions(+), 27 deletions(-)
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 6fc03af..775e05f 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -229,6 +229,16 @@ STUB_SET_VARIABLE(virt, id)
STUB_GET_NEXT_HIGH_MONO_COUNT(virt, id)
STUB_RESET_SYSTEM(virt, id)
+efi_status_t efi_get_time(efi_time_t *tm, efi_time_cap_t *cap)
+{
+ return efi.get_time(tm, cap);
+}
+
+efi_status_t efi_set_time(efi_time_t *tm)
+{
+ return efi.set_time(tm);
+}
+
void
efi_gettimeofday (struct timespec *ts)
{
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 3ae4128..735a470 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -89,24 +89,12 @@ early_param("add_efi_memmap", setup_add_efi_memmap);
static efi_status_t virt_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc)
{
- unsigned long flags;
- efi_status_t status;
-
- spin_lock_irqsave(&rtc_lock, flags);
- status = efi_call_virt2(get_time, tm, tc);
- spin_unlock_irqrestore(&rtc_lock, flags);
- return status;
+ return efi_call_virt2(get_time, tm, tc);
}
static efi_status_t virt_efi_set_time(efi_time_t *tm)
{
- unsigned long flags;
- efi_status_t status;
-
- spin_lock_irqsave(&rtc_lock, flags);
- status = efi_call_virt1(set_time, tm);
- spin_unlock_irqrestore(&rtc_lock, flags);
- return status;
+ return efi_call_virt1(set_time, tm);
}
static efi_status_t virt_efi_get_wakeup_time(efi_bool_t *enabled,
@@ -232,14 +220,11 @@ static efi_status_t __init phys_efi_set_virtual_address_map(
static efi_status_t __init phys_efi_get_time(efi_time_t *tm,
efi_time_cap_t *tc)
{
- unsigned long flags;
efi_status_t status;
- spin_lock_irqsave(&rtc_lock, flags);
efi_call_phys_prelog();
status = efi_call_phys2(efi_phys.get_time, tm, tc);
efi_call_phys_epilog();
- spin_unlock_irqrestore(&rtc_lock, flags);
return status;
}
@@ -250,6 +235,8 @@ int efi_set_rtc_mmss(unsigned long nowtime)
efi_time_t eft;
efi_time_cap_t cap;
+ assert_spin_locked(&rtc_lock);
+
status = efi.get_time(&eft, &cap);
if (status != EFI_SUCCESS) {
printk(KERN_ERR "Oops: efitime: can't read time!\n");
@@ -272,12 +259,14 @@ int efi_set_rtc_mmss(unsigned long nowtime)
return 0;
}
-unsigned long efi_get_time(void)
+unsigned long efi_get_time_locked(void)
{
efi_status_t status;
efi_time_t eft;
efi_time_cap_t cap;
+ assert_spin_locked(&rtc_lock);
+
status = efi.get_time(&eft, &cap);
if (status != EFI_SUCCESS)
printk(KERN_ERR "Oops: efitime: can't read time!\n");
@@ -286,6 +275,28 @@ unsigned long efi_get_time(void)
eft.minute, eft.second);
}
+efi_status_t efi_get_time(efi_time_t *tm, efi_time_cap_t *cap)
+{
+ unsigned long flags;
+ efi_status_t status;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ status = efi.get_time(tm, cap);
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ return status;
+}
+
+efi_status_t efi_set_time(efi_time_t *tm)
+{
+ unsigned long flags;
+ efi_status_t status;
+
+ spin_lock_irqsave(&rtc_lock, flags);
+ status = efi.set_time(tm);
+ spin_unlock_irqrestore(&rtc_lock, flags);
+ return status;
+}
+
/*
* Tell the kernel about the EFI memory map. This might include
* more than the max 128 entries that can fit in the e820 legacy
@@ -571,7 +582,7 @@ void __init efi_init(void)
do_add_efi_memmap();
#ifdef CONFIG_X86_32
- x86_platform.get_wallclock = efi_get_time;
+ x86_platform.get_wallclock = efi_get_time_locked;
x86_platform.set_wallclock = efi_set_rtc_mmss;
#endif
diff --git a/drivers/char/efirtc.c b/drivers/char/efirtc.c
index 53c524e..cdb1bed 100644
--- a/drivers/char/efirtc.c
+++ b/drivers/char/efirtc.c
@@ -174,7 +174,7 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
case RTC_RD_TIME:
spin_lock_irqsave(&efi_rtc_lock, flags);
- status = efi.get_time(&eft, &cap);
+ status = efi_get_time(&eft, &cap);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
@@ -201,7 +201,7 @@ static long efi_rtc_ioctl(struct file *file, unsigned int cmd,
spin_lock_irqsave(&efi_rtc_lock, flags);
- status = efi.set_time(&eft);
+ status = efi_set_time(&eft);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
@@ -312,7 +312,7 @@ efi_rtc_get_status(char *buf)
spin_lock_irqsave(&efi_rtc_lock, flags);
- efi.get_time(&eft, &cap);
+ efi_get_time(&eft, &cap);
efi.get_wakeup_time(&enabled, &pending, &alm);
spin_unlock_irqrestore(&efi_rtc_lock,flags);
diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
index 5502923..6307f5c 100644
--- a/drivers/rtc/rtc-efi.c
+++ b/drivers/rtc/rtc-efi.c
@@ -153,7 +153,7 @@ static int efi_read_time(struct device *dev, struct rtc_time *tm)
efi_time_t eft;
efi_time_cap_t cap;
- status = efi.get_time(&eft, &cap);
+ status = efi_get_time(&eft, &cap);
if (status != EFI_SUCCESS) {
/* should never happen */
@@ -166,21 +166,21 @@ static int efi_read_time(struct device *dev, struct rtc_time *tm)
return rtc_valid_tm(tm);
}
-static int efi_set_time(struct device *dev, struct rtc_time *tm)
+static int rtc_efi_set_time(struct device *dev, struct rtc_time *tm)
{
efi_status_t status;
efi_time_t eft;
convert_to_efi_time(tm, &eft);
- status = efi.set_time(&eft);
+ status = efi_set_time(&eft);
return status == EFI_SUCCESS ? 0 : -EINVAL;
}
static const struct rtc_class_ops efi_rtc_ops = {
.read_time = efi_read_time,
- .set_time = efi_set_time,
+ .set_time = rtc_efi_set_time,
.read_alarm = efi_read_alarm,
.set_alarm = efi_set_alarm,
};
diff --git a/include/linux/efi.h b/include/linux/efi.h
index c5709ed..57cd963 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -454,7 +454,9 @@ extern u64 efi_mem_attribute (unsigned long phys_addr, unsigned long size);
extern int __init efi_uart_console_only (void);
extern void efi_initialize_iomem_resources(struct resource *code_resource,
struct resource *data_resource, struct resource *bss_resource);
-extern unsigned long efi_get_time(void);
+extern unsigned long efi_get_time_locked(void);
+extern efi_status_t efi_get_time(efi_time_t *tm, efi_time_cap_t *cap);
+extern efi_status_t efi_set_time(efi_time_t *tm);
extern int efi_set_rtc_mmss(unsigned long nowtime);
extern void efi_reserve_boot_services(void);
extern struct efi_memory_map memmap;
--
1.7.4.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists