linux-kernel - Re: Broken locking in leds-lp5523.c

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: <20140109230807.GA19974@amd.pavel.ucw.cz>
Date:	Fri, 10 Jan 2014 00:08:08 +0100
From:	Pavel Machek <pavel@....cz>
To:	Bryan Wu <cooloney@...il.com>
Cc:	milo kim <milo.kim@...com>,
	"rpurdie@...ys.net" <rpurdie@...ys.net>,
	Linux LED Subsystem <linux-leds@...r.kernel.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Pali Rohár <pali.rohar@...il.com>,
	Sebastian Reichel <sre@...ian.org>,
	Sebastian Reichel <sre@...g0.de>
Subject: Re: Broken locking in leds-lp5523.c

Hi!

> > There's some locking weirdness, and few missing comments in lp5523
> > driver.
> >
> > Now, this is untested patch from my reverse-engineering. I hope I
> > understood things right...
> >
> > In particular, there's unbalanced unlock in
> > lp5523_update_program_memory, and lp5523_update_program_memory needs
> > to be protected by the lock.
> >
> > Comments? Does someone maintain this?
> 
> I think Milo Kim is maintaining this driver for the LP55xx chip
 > family.

Yes, he's on the Cc list. I was hoping he would comment.

> > -#define LP5523_PROGRAM_LENGTH          32
> > +#define LP5523_PROGRAM_LENGTH          32      /* bytes */
> > +/* Memory is used like this:
> > +   0x00 engine 1 program
> > +   0x10 engine 2 program
> > +   0x20 engine 3 program
> > +   0x30 engine 1 muxing info
> > +   0x40 engine 2 muxing info
> > +   0x50 engine 3 muxing info
> > +   ...and offsets are hard-coded all around :-( */

For example, I'd like to increase limit here -- 62 byte programs
should be feasible AFAICT.

> >  #define LP5523_MAX_LEDS                        9
> >
> >  /* Registers */
> > @@ -265,20 +273,25 @@ static int lp5523_init_program_engine(struct lp55xx_chip *chip)
> >         /* one pattern per engine setting LED MUX start and stop addresses */
> >         static const u8 pattern[][LP5523_PROGRAM_LENGTH] =  {
> >                 { 0x9c, 0x30, 0x9c, 0xb0, 0x9d, 0x80, 0xd8, 0x00, 0},
> > +               /* 9c30 -- mux_map_start(0x30)
> > +                  9cb0 -- mux_ld_end(0x50)
> > +                  9d80 -- mux_sel???(0x00)
> > +                  d800 -- invalid?? */

And it would be good to get comment on what this does. I could not
disassemble/decipher it.

Regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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/