linux-kernel - Re: [PATCH] Avoid null-pointer access in w1/slaves/w1

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:	Tue, 10 Mar 2015 01:05:30 +0100
From:	Thorsten Bschorr <thorsten@...horr.de>
To:	David Fries <david@...es.net>
Cc:	"??????? ???????" <zbr@...emap.net>,
	Jonathan ALIBERT <jonathan.alibert@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Avoid null-pointer access in w1/slaves/w1_therm

> Oops, sorry, it got lost in the shuffle, here's the first patch again
> (the others were for debugging and increase that time and so wouldn't
> go upstream anyway).

I assumed so.


It looks like your patch runs into dead locks problems:

I have a cron job reading my sensors. If I read the sensors on another
thread (e.g. via cat), the 2nd thread can produce a dead lock:

* thread 1 has bus & sl lock
* thread 1 drops bus lock, keeps sl locks and sleeps
* thread 2 get bus lock, waits for sl lock
* thread 1 returns from sleep, waits for bus lock, but this is help by thread 2


After a couple of seconds, I get the following dump (on 3.18.9):

Mar 10 00:29:33 pi kernel: [  481.184239] cat             D c0546c48
  0  2523   2422 0x00000000
Mar 10 00:29:33 pi kernel: [  481.184320] [<c0546c48>] (__schedule)
from [<c0547130>] (schedule+0x40/0x8c)
Mar 10 00:29:33 pi kernel: [  481.184357] [<c0547130>] (schedule) from
[<c05473f0>] (schedule_preempt_disabled+0x30/0x40)
Mar 10 00:29:33 pi kernel: [  481.184393] [<c05473f0>]
(schedule_preempt_disabled) from [<c0548dc8>]
(__mutex_lock_slowpath+0xa8/0x174)
Mar 10 00:29:33 pi kernel: [  481.184424] [<c0548dc8>]
(__mutex_lock_slowpath) from [<c0548ecc>] (mutex_lock+0x38/0x3c)
Mar 10 00:29:33 pi kernel: [  481.184465] [<c0548ecc>] (mutex_lock)
from [<bf0220f4>] (w1_slave_show+0x60/0x3ec [w1_therm])
Mar 10 00:29:33 pi kernel: [  481.184517] [<bf0220f4>] (w1_slave_show
[w1_therm]) from [<c03565bc>] (dev_attr_show+0x2c/0x58)
Mar 10 00:29:33 pi kernel: [  481.184558] [<c03565bc>] (dev_attr_show)
from [<c01a5f54>] (sysfs_kf_seq_show+0x9c/0x120)
Mar 10 00:29:33 pi kernel: [  481.184589] [<c01a5f54>]
(sysfs_kf_seq_show) from [<c01a479c>] (kernfs_seq_show+0x34/0x38)
Mar 10 00:29:33 pi kernel: [  481.184624] [<c01a479c>]
(kernfs_seq_show) from [<c015905c>] (seq_read+0x1ac/0x4c4)
Mar 10 00:29:33 pi kernel: [  481.184653] [<c015905c>] (seq_read) from
[<c01a52d4>] (kernfs_fop_read+0x11c/0x164)
Mar 10 00:29:33 pi kernel: [  481.184691] [<c01a52d4>]
(kernfs_fop_read) from [<c0135a98>] (vfs_read+0x98/0x18c)
Mar 10 00:29:33 pi kernel: [  481.184721] [<c0135a98>] (vfs_read) from
[<c0135bd8>] (SyS_read+0x4c/0x98)
Mar 10 00:29:33 pi kernel: [  481.184758] [<c0135bd8>] (SyS_read) from
[<c000eb40>] (ret_fast_syscall+0x0/0x48)


In principle, w1_slave_show  only needs a read-lock on the sl data,
and only w1_therm_remove_slave needs a write-lock. This would allow
multiple concurrent temperature readings (as before).


BTW: in w1_therm_add_slave the mutex_init call should be after if
(!sl->family_data), otherwise one might get another null pointer
issue.
--
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/