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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bc1a086b932f454f9379c49221983675@xiaomi.com>
Date: Mon, 1 Jul 2024 02:11:36 +0000
From: 朱恺乾 <zhukaiqian@...omi.com>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, Thomas Gleixner
	<tglx@...utronix.de>, 张嘉伟 <zhangjiawei8@...omi.com>
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>, 梁伟鹏
	<weipengliang@...omi.com>, 翁金飞 <wengjinfei@...omi.com>
Subject: RE: [External Mail]Re: Race condition when replacing the broadcast
 timer

+Jiawei

Jiawei,
Please update here when you have the test result


-----Original Message-----
From: Daniel Lezcano <daniel.lezcano@...aro.org>
Sent: Friday, June 28, 2024 3:23 PM
To: 朱恺乾 <zhukaiqian@...omi.com>; Thomas Gleixner <tglx@...utronix.de>
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: Re: [External Mail]Re: Race condition when replacing the broadcast timer

[外部邮件] 此邮件来源于小米公司外部,请谨慎处理。若对邮件安全性存疑,请将邮件转发给misec@...omi.com进行反馈

On 28/06/2024 03:59, 朱恺乾 wrote:
> Thanks for the fast reply.
> May I know when there'll be a formal patch on the mainline?

Do you confirm the patch fixes the issue ?


> -----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!******/#

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog

#/******本邮件及其附件含有小米公司的保密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本邮件! 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

Powered by Openwall GNU/*/Linux Powered by OpenVZ