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]
Date:	Sat, 6 Jun 2009 20:47:36 +0800
From:	Feng Tang <feng.tang@...el.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	"mingo@...e.hu" <mingo@...e.hu>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tick: add check for the existence of broadcast clock
 event device

On Sat, 6 Jun 2009 17:24:50 +0800
Thomas Gleixner <tglx@...utronix.de> wrote:

> On Fri, 5 Jun 2009, Feng Tang wrote:
> 
> > >From 2f076e1867c8bbb145b74d289358174644d9fed8 Mon Sep 17 00:00:00
> > >2001
> > From: Feng Tang <feng.tang@...el.com>
> > Date: Fri, 5 Jun 2009 10:36:15 +0800
> > Subject: [PATCH] tick: add check for the existence of broadcast
> > clock event device
> > 
> > Some platform may have no broadcast clock event device, as it use
> > always-on external timers for per-cpu timer and has no extra one
> > for broadcast device. This check will secure the access to bc
> > device when system get some boradcast on/off and enter/exit message
> >
> > Signed-off-by: Feng Tang <feng.tang@...el.com>
> > ---
> >  kernel/time/tick-broadcast.c |    8 +++++++-
> >  1 files changed, 7 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/time/tick-broadcast.c
> > b/kernel/time/tick-broadcast.c index 118a3b3..110e0bc 100644
> > --- a/kernel/time/tick-broadcast.c
> > +++ b/kernel/time/tick-broadcast.c
> > @@ -214,10 +214,13 @@ static void tick_do_broadcast_on_off(void
> > *why) 
> >  	spin_lock_irqsave(&tick_broadcast_lock, flags);
> >  
> > +	bc = tick_broadcast_device.evtdev;
> > +	if (!bc)
> > +		goto out;
> > +
> 
> This check is not necessary because we check whether the percpu device
> is affected by the stops in C3 madness _before_ we touch the broadcast
> device.
> 
> 	 if (!dev || !(dev->features & CLOCK_EVT_FEAT_C3STOP))
> 	    	  got out;
> 
> If your percpu devices are always on (not affected by C3 stop) then
> you never dereference bc. So why do we need an extra check for !bc ?

Hi tglx,

Thanks for the explanation. But we really ran into the NULL pointer case, in our platform, there are 2 X86 CPUs which have lapic, also it has 2 external timers which are pretty similar with HPET timers, those 2 external timers will be used as per-cpu timers (higher rating than lapic timer). In system's power cycle of suspend and resume, disable_nontboot_cpus will be called before goto suspend state,and enable_nonboot_cpus will be called for the resume process, so lapic timer of cpu1 will be first registered as per-cpu timer, and our external timer will be registered later after get a CPU_ONLINE notifier (similar with HPET), right in this time slot that lapic is the per-cpu timer, when system get the CLOCK_EVT_BROADCAST_ENTER/EXIT msg, tick_do_broadcast_on_off() is called and hit the NULL pointer case.

Our external timer driver is very similar with HPET dirver, why HPET doesn't see such an issue? becuase HPET has enough number of timers, and it use "hpet0" as the bc device, while our platform doesn't have a extra one to act as bc.

- Feng 




> 
> Thanks,
> 
> 	tglx
--
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