linux-kernel - Re: [patch 00/15] clocksource / timekeeping rework V4 (resend V3 + bug fix)

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: <alpine.LFD.2.00.0908171038420.2782@localhost.localdomain>
Date:	Mon, 17 Aug 2009 10:45:00 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Martin Schwidefsky <schwidefsky@...ibm.com>
cc:	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	john stultz <johnstul@...ibm.com>, linux-kernel@...r.kernel.org
Subject: Re: [patch 00/15] clocksource / timekeeping rework V4 (resend V3 +
 bug fix)

On Mon, 17 Aug 2009, Martin Schwidefsky wrote:
> > -	spin_unlock(&watchdog_lock);
> > +	spin_unlock_irqrestore(&watchdog_lock, flags);
> > +
> > +	/* Needs to be done outside of watchdog lock */
> > +	list_for_each_entry_safe(cs, tmp, &unstable, wd_list) {
> > +		list_del_init(&cs->wd_list);
> > +		clocksource_change_rating(cs, 0);
> > +	}
> >  }
> > 
> >  #else /* CONFIG_CLOCKSOURCE_WATCHDOG */
> 
> Autsch, the s/spin_unlock/spin_unlock_irqrestore/ is rather obvious, no?

:)

> In theory we now have a reference count problem. The call to 
> clocksource_change_rating are no done outside of the clocksource mutex
> and outside of the watchdog lock. A concurrent clocksource_unregister
> could remove the clock.

Darn, yes. But it's not hard to fix.

Thanks,

	tglx
---
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 02dc22d..c6bff11 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -131,6 +131,7 @@ static cycle_t watchdog_last;
 static int watchdog_running;
 
 static void clocksource_watchdog_work(struct work_struct *work);
+static void __clocksource_change_rating(struct clocksource *cs, int rating);
 
 /*
  * Interval: 0.5sec Threshold: 0.0625s
@@ -309,6 +310,7 @@ static void clocksource_watchdog_work(struct work_struct *work)
 	unsigned long flags;
 	LIST_HEAD(unstable);
 
+	mutex_lock(&clocksource_mutex);
 	spin_lock_irqsave(&watchdog_lock, flags);
 	list_for_each_entry_safe(cs, tmp, &watchdog_list, wd_list)
 		if (cs->flags & CLOCK_SOURCE_UNSTABLE) {
@@ -322,8 +324,9 @@ static void clocksource_watchdog_work(struct work_struct *work)
 	/* Needs to be done outside of watchdog lock */
 	list_for_each_entry_safe(cs, tmp, &unstable, wd_list) {
 		list_del_init(&cs->wd_list);
-		clocksource_change_rating(cs, 0);
+		__clocksource_change_rating(cs, 0);
 	}
+	mutex_unlock(&clocksource_mutex);
 }
 
 #else /* CONFIG_CLOCKSOURCE_WATCHDOG */
@@ -470,16 +473,21 @@ int clocksource_register(struct clocksource *cs)
 }
 EXPORT_SYMBOL(clocksource_register);
 
+static void __clocksource_change_rating(struct clocksource *cs, int rating)
+{
+	list_del(&cs->list);
+	cs->rating = rating;
+	clocksource_enqueue(cs);
+	clocksource_select();
+}
+
 /**
  * clocksource_change_rating - Change the rating of a registered clocksource
  */
 void clocksource_change_rating(struct clocksource *cs, int rating)
 {
 	mutex_lock(&clocksource_mutex);
-	list_del(&cs->list);
-	cs->rating = rating;
-	clocksource_enqueue(cs);
-	clocksource_select();
+	__clocksource_change_rating(cs, rating);
 	mutex_unlock(&clocksource_mutex);
 }
 EXPORT_SYMBOL(clocksource_change_rating);
--
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/