| 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: Fri, 28 Jun 2024 01:59:03 +0000
From: 朱恺乾 <zhukaiqian@...omi.com>
To: Thomas Gleixner <tglx@...utronix.de>, Daniel Lezcano
<daniel.lezcano@...aro.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
王韬 <lingyue@...omi.com>, 熊亮
<xiongliang@...omi.com>, "isaacmanjarres@...gle.com"
<isaacmanjarres@...gle.com>, Frederic Weisbecker <frederic@...nel.org>,
Anna-Maria Behnsen <anna-maria@...utronix.de>
Subject: RE: [External Mail]Re: Race condition when replacing the broadcast
timer
Thanks for the fast reply.
May I know when there'll be a formal patch on the mainline?
-----Original Message-----
From: Thomas Gleixner <tglx@...utronix.de>
Sent: Thursday, June 27, 2024 7:27 PM
To: 朱恺乾 <zhukaiqian@...omi.com>; Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: linux-kernel@...r.kernel.org; 王韬 <lingyue@...omi.com>; 熊亮 <xiongliang@...omi.com>; isaacmanjarres@...gle.com; Frederic Weisbecker <frederic@...nel.org>; Anna-Maria Behnsen <anna-maria@...utronix.de>
Subject: [External Mail]Re: Race condition when replacing the broadcast timer
[外部邮件] 此邮件来源于小米公司外部，请谨慎处理。若对邮件安全性存疑，请将邮件转发给misec@...omi.com进行反馈
On Wed, Jun 26 2024 at 02:17, 朱恺乾 wrote:
> We find a possible race condition when replacing the broadcast timer.
> Here is how the race happend,
> 1. In thread 0, ___tick_broadcast_oneshot_control, timer 0 as a
> broadcast timer is updating the next_event.
> 2. In thread 1, tick_install_broadcast_device, timer 0 is going to be
> replaced by a new timer 1.
> 3. If thread 0 gets the broadcast timer first, it would have the old
> timer returned (timer 0). When thread 1 shuts the old timer down and
> marks it as detached, Thread 0 still have the chance to re-enable the
> old timer with a noop handler if it executes slower than thread 1.
> 4. As the old timer is binded to a CPU, when plug out that CPU, kernel
> fails at clockevents.c:653
Clearly tick_install_broadcast_device() lacks serialization.
The untested patch below should cure that.
Thanks,
tglx
---
kernel/time/clockevents.c | 31 +++++++++++++++++++------------
kernel/time/tick-broadcast.c | 36 ++++++++++++++++++++++--------------
kernel/time/tick-internal.h | 2 ++
3 files changed, 43 insertions(+), 26 deletions(-)
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -557,23 +557,14 @@ void clockevents_handle_noop(struct cloc { }
-/**
- * clockevents_exchange_device - release and request clock devices
- * @old: device to release (can be NULL)
- * @new: device to request (can be NULL)
- *
- * Called from various tick functions with clockevents_lock held and
- * interrupts disabled.
- */
-void clockevents_exchange_device(struct clock_event_device *old,
- struct clock_event_device *new)
+void __clockevents_exchange_device(struct clock_event_device *old,
+ struct clock_event_device *new)
{
/*
* Caller releases a clock event device. We queue it into the
* released list and do a notify add later.
*/
if (old) {
- module_put(old->owner);
clockevents_switch_state(old, CLOCK_EVT_STATE_DETACHED);
list_move(&old->list, &clockevents_released);
}
@@ -585,6 +576,22 @@ void clockevents_exchange_device(struct
}
/**
+ * clockevents_exchange_device - release and request clock devices
+ * @old: device to release (can be NULL)
+ * @new: device to request (can be NULL)
+ *
+ * Called from various tick functions with clockevents_lock held and
+ * interrupts disabled.
+ */
+void clockevents_exchange_device(struct clock_event_device *old,
+ struct clock_event_device *new) {
+ if (old)
+ module_put(old->owner);
+ __clockevents_exchange_device(old, new); }
+
+/**
* clockevents_suspend - suspend clock devices
*/
void clockevents_suspend(void)
@@ -650,7 +657,7 @@ void tick_cleanup_dead_cpu(int cpu)
if (cpumask_test_cpu(cpu, dev->cpumask) &&
cpumask_weight(dev->cpumask) == 1 &&
!tick_is_broadcast_device(dev)) {
- BUG_ON(!clockevent_state_detached(dev));
+ WARN_ON(!clockevent_state_detached(dev));
list_del(&dev->list);
}
}
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -162,23 +162,31 @@ static bool tick_set_oneshot_wakeup_devi
*/
void tick_install_broadcast_device(struct clock_event_device *dev, int cpu) {
- struct clock_event_device *cur = tick_broadcast_device.evtdev;
+ struct clock_event_device *cur;
- if (tick_set_oneshot_wakeup_device(dev, cpu))
- return;
+ scoped_guard(raw_spinlock_irqsave, &tick_broadcast_lock) {
- if (!tick_check_broadcast_device(cur, dev))
- return;
+ if (tick_set_oneshot_wakeup_device(dev, cpu))
+ return;
- if (!try_module_get(dev->owner))
- return;
+ cur = tick_broadcast_device.evtdev;
+ if (!tick_check_broadcast_device(cur, dev))
+ return;
- clockevents_exchange_device(cur, dev);
+ if (!try_module_get(dev->owner))
+ return;
+
+ __clockevents_exchange_device(cur, dev);
+ if (cur)
+ cur->event_handler = clockevents_handle_noop;
+ WRITE_ONCE(tick_broadcast_device.evtdev, dev);
+ if (!cpumask_empty(tick_broadcast_mask))
+ tick_broadcast_start_periodic(dev);
+ }
+
+ /* Module release must be outside of the lock */
if (cur)
- cur->event_handler = clockevents_handle_noop;
- tick_broadcast_device.evtdev = dev;
- if (!cpumask_empty(tick_broadcast_mask))
- tick_broadcast_start_periodic(dev);
+ module_put(old->owner);
if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT))
return;
@@ -1185,7 +1193,7 @@ int tick_broadcast_oneshot_active(void)
*/
bool tick_broadcast_oneshot_available(void)
{
- struct clock_event_device *bc = tick_broadcast_device.evtdev;
+ struct clock_event_device *bc =
+ READ_ONCE(tick_broadcast_device.evtdev);
return bc ? bc->features & CLOCK_EVT_FEAT_ONESHOT : false; } @@ -1193,7 +1201,7 @@ bool tick_broadcast_oneshot_available(vo
#else
int __tick_broadcast_oneshot_control(enum tick_broadcast_state state) {
- struct clock_event_device *bc = tick_broadcast_device.evtdev;
+ struct clock_event_device *bc =
+ READ_ONCE(tick_broadcast_device.evtdev);
if (!bc || (bc->features & CLOCK_EVT_FEAT_HRTIMER))
return -EBUSY;
--- a/kernel/time/tick-internal.h
+++ b/kernel/time/tick-internal.h
@@ -53,6 +53,8 @@ static inline void clockevent_set_state( }
extern void clockevents_shutdown(struct clock_event_device *dev);
+extern void __clockevents_exchange_device(struct clock_event_device *old,
+ struct clock_event_device
+*new);
extern void clockevents_exchange_device(struct clock_event_device *old,
struct clock_event_device *new); extern void clockevents_switch_state(struct clock_event_device *dev,
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#
Powered by blists - more mailing lists