linux-kernel - Re: [PATCH] Input: synaptics-rmi4 - create firmware update sysfs attribute in F34

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] [day] [month] [year] [list]

Date:   Mon, 13 Feb 2017 21:41:27 +0000
From:   Nick Dyer <nick@...anahar.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-input@...r.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>,
        Andrew Duggan <aduggan@...aptics.com>,
        Christopher Heiny <cheiny@...aptics.com>,
        linux-kernel@...r.kernel.org, Chris Healy <cphealy@...il.com>
Subject: Re: [PATCH] Input: synaptics-rmi4 - create firmware update sysfs
 attribute in F34

On Sun, Feb 12, 2017 at 04:02:51PM -0800, Dmitry Torokhov wrote:
> On Sun, Feb 12, 2017 at 10:50:56PM +0000, Nick Dyer wrote:
> > On Thu, Feb 09, 2017 at 01:25:08PM -0800, Dmitry Torokhov wrote:
> > > There is no need to create sysfs attributes in the main driver core,
> > > let F34 implementation do that.
> > 
> > I haven't tested this yet, but I did try creating/removing the sysfs
> > entries in the f34 function probe/remove as you're suggesting when I
> > wrote the F34 support.
> > 
> > The problem is that when we do a firmware update, we have to tear down
> > the function list (because most of the functions are not there during
> > firmware update and they may in fact come back different). Which removes
> > the sysfs entries, meaning it's rather like sawing off the branch you're
> > sitting on, and it crashes almost immediately. I couldn't think of a
> > cleaner way to solve it that the current implementation.
> 
> Oh, I see. Well, that means that the current implementation is quite
> broken, as you'll end up referencing freed and potentially reused memory
> after firmware update. It looks like we'll need to make sure we do not
> reference F34 memory unless we know it is there. That means we'll have
> to pull update status and FW update mutex into the RMI device itself as
> it is something that stays around even if we destroy and rebuild
> function list.

I see what you mean. The code does check that f34_container isn't null
(eg in rmi_driver_configuration_id_show) but since it may be accessed
from multiple threads it needs a mutex around it.