linux-kernel - Re: [PATCH] applesmc - fix crash when activating a led trigger on the keyboard backlight

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 14 Apr 2007 09:45:11 +0100
From:	Richard Purdie <rpurdie@...ys.net>
To:	Nicolas Boichat <nicolas@...chat.ch>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Bradley Hook <bdhook@...il.com>,
	mactel-linux-devel@...ts.sourceforge.net,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] applesmc - fix crash when activating a led trigger on
	the keyboard backlight

Hi,

On Sat, 2007-04-14 at 16:05 +0800, Nicolas Boichat wrote:
> Bradley Hook wrote:
> > Slightly off-topic, but I've been experiencing a minor bug in the
> > keyboard backlight feature.
> >
> > I say it is "minor" only because the feature serves no real functional
> > purpose. You can activate a trigger called "heartbeat" that will cause
> > the keyboard light to pulse at a speed based on the CPU usage. On my
> > MBP17, after activating this trigger the machine will either lock-up
> > or core dump within about a minute (timing is not consistent).
> >   
> 
> This is caused by the fact applesmc_backlight_set locks a mutex (or more
> precisely sleeps while trying to lock a mutex) while being in a softirq
> context.
> 
> This might be obvious for others, but it was not for me, and there is
> absolutely no mention in the documentation of the fact it is not always
> safe to sleep in the brightness_set handler of a led_class device (it is
> safe when it is called because someone wrote to the brightness sysfs file).

Its never safe for a brightness_set handler to sleep. They're designed
to be called from interrupt context and as you've noted, several
triggers do that.

The solution if you have locks like your case is to offload the work to
a workqueue, there is simply no other way to do it.

I'll have a look to see if I can improve the documentation (patches
welcome).

> Also, the led-trigger code seems buggy when it comes to locking. Setting
> CONFIG_DEBUG_SPINLOCK_SLEEP causes a lot a warnings. The problem is that
> the list of triggers is locked using a rw spinlock, but the rest of the
> code seems to ignore that, and calls a lot of functions which can sleep
> (kzalloc with GFP_KERNEL, sysfs_add_file, mutex_lock, etc...). I think
> the list lock should be converted to a mutex (or maybe modified to use
> RCU). I'm not very experienced in that domain, but if you want I can
> provide a patch for this.

Someone else has mentioned this within the past few days and there is a
problem with the trigger->activate and ->deactivate calls occurring
within a spinlock. Its not a simple problem to solve unfortunately and
you can't just convert to a mutex but I'm looking at it. Again, patches
welcome but its going to need careful thought.

Cheers,

Richard


-
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/