linux-kernel - Re: Unregistering extcon providers while they are in use leads to kernel crashes

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]

Message-id: <5861F678.9010408@samsung.com>
Date:   Tue, 27 Dec 2016 14:04:56 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     myungjoo.ham@...sung.com,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Unregistering extcon providers while they are in use leads to
 kernel crashes

Hi Hans,

Thanks for your report.
I'll check this problem and try to resolve it.

On 2016년 12월 22일 00:54, Hans de Goede wrote:
> Hi,
> 
> With the recent extcon work I've been doing I noticed that
> if I want to rmmod and then insmod say extcon_axp288 I can
> do so without problems even if axp288_charger is holding
> a reference to the extcon device returned by extcon_get_extcon_dev.
> 
> The problem is that extcon_get_extcon_dev simply looks up
> the extcon-device in the list of current registered extcon-s
> and then returns a pointer to it, without any reference
> counting.
> 
> The rmmod scenario can be fixed by doing a module_get from
> extcon_get_extcon_dev, but that still leaves the same problem
> when root manually unbinds the driver through sysfs.
> 
> A possible way fix this would be:
> 
> 1) Make all extcon providers use devm_extcon_dev_allocate and document
> using this to allocate an extcon_dev mandatory
> 
> 2) Add a refcount to struct extcon_dev and introduce extcon_dev_get
> and extcon_dev_put helpers which modify the refcount and only free
> the memory on the final put (and make the evm_extcon_dev_allocate
> cleanup function call extcon_dev_put)
> 
> 3) On extcon_dev_unregister set a flag in the extcon_dev that it
> has been free-ed, make all extcon consumer functions which take
> an extcon_dev (extcon_get_state, extcon_register_notifier, etc.)
> check this flag and return -ENODEV when the extcon has been unregistered
> 
> 4) Make extcon_get_extcon_dev call extcon_dev_get on the returned edev
> before returning it
> 
> From here on we've fixed the crash, but we now leak the extcon_dev
> when the consumer gets unbound.
> 
> 5) Add a devm_extcon_get_extcon_dev which calls extcon_dev_put as the devm
> cleanup function
> 
> 6) Convert all extcon consumers to use devm_extcon_get_extcon_dev
> 
> Regards,
> 
> Hans
> 
> 
> 

-- 
Regards,
Chanwoo Choi