linux-kernel - Re: applesmc oops in 3.10/3.11

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:	Wed, 25 Sep 2013 15:08:38 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	Josh Boyer <jwboyer@...oraproject.org>, khali@...ux-fr.org,
	lm-sensors@...sensors.org,
	"Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
	bugzilla@...orremedies.com
Subject: Re: applesmc oops in 3.10/3.11

On Wed, Sep 25, 2013 at 11:48:07PM +0200, Henrik Rydberg wrote:
> On Wed, Sep 25, 2013 at 12:56:28PM -0700, Guenter Roeck wrote:
> > On Wed, Sep 25, 2013 at 03:06:25PM -0400, Josh Boyer wrote:
> > > Hi All,
> > > 
> > > Chris has reported[1] an issue with the applesmc driver in the 3.10
> > > and 3.11 kernels.  This seems to be a bit transient, but the oops is
> > > consistent across those releases.  This is with a MacBook Pro 4,1.
> > > The backtrace is below.
> > > 
> > > Any ideas on this one?
> > > 
> > Some of the bug reports suggest that there are related messages in at least some
> > of the kernel logs.
> > 
> > Jul 02 22:23:22 f19s.local kernel: applesmc: send_byte(0x04, 0x0300) fail: 0x01
> > Jul 02 22:23:22 f19s.local kernel: applesmc: : read len fail
> > 
> > This suggests that initialization may be attempted more than once. The key cache
> > is allocated only once, but the number of keys is read for each attempt.
> > 
> > No idea if that can happen, but if the number of keys can increase after
> > the first initialization attempt you would have an explanation for the crash.
> 
> Good idea, and easy enough to test with the patch below.
> 
Should we apply this patch even though it may not solve the specific problem ?

Again, not sure if the key count can change, but the current code is at the very
least inconsistent, as it keeps reading the key count without updating or
verifying the cache size.

Thanks,
Guenter

> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 62c2e32..d962c4d 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -525,16 +525,24 @@ static int applesmc_init_smcreg_try(void)
>  {
>         struct applesmc_registers *s = &smcreg;
>         bool left_light_sensor, right_light_sensor;
> +       unsigned int count;
>         u8 tmp[1];
>         int ret;
>  
>         if (s->init_complete)
>                 return 0;
>  
> -       ret = read_register_count(&s->key_count);
> +       ret = read_register_count(&count);
>         if (ret)
>                 return ret;
>  
> +       if (s->cache && s->key_count != count) {
> +               pr_warn("key count changed from %d to %d\n", s->key_count, count);
> +               kfree(s->cache);
> +               s->cache = NULL;
> +       }
> +       s->key_count = count;
> +
>         if (!s->cache)
>                 s->cache = kcalloc(s->key_count, sizeof(*s->cache), GFP_KERNEL);
>         if (!s->cache)
>  
> Thanks,
> Henrik
> 
--
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/