Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754096AbbG3AKd (ORCPT ); Wed, 29 Jul 2015 20:10:33 -0400 Received: from mail-pd0-f182.google.com ([209.85.192.182]:33441 "EHLO mail-pd0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752153AbbG3AKb (ORCPT ); Wed, 29 Jul 2015 20:10:31 -0400 Date: Wed, 29 Jul 2015 17:10:26 -0700 From: Dmitry Torokhov To: Krzysztof Kozlowski Cc: Jiri Kosina , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, sre@kernel.org, linux-pm@vger.kernel.org, "H.J. Lu" , stable@vger.kernel.org Subject: Re: [PATCH] HID: hid-input: Fix accessing freed memory during driver unbind Message-ID: <20150730001026.GH23178@dtor-ws> References: <1438042590-17719-1-git-send-email-k.kozlowski@samsung.com> <20150729174619.GC23178@dtor-ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2441 Lines: 61 On Thu, Jul 30, 2015 at 08:42:12AM +0900, Krzysztof Kozlowski wrote: > 2015-07-30 2:46 GMT+09:00 Dmitry Torokhov : > > On Wed, Jul 29, 2015 at 03:07:04PM +0200, Jiri Kosina wrote: > >> On Tue, 28 Jul 2015, Krzysztof Kozlowski wrote: > >> > >> > During unbinding the driver was dereferencing a pointer to memory > >> > already freed by power_supply_unregister(). > >> > > >> > Driver was freeing its internal description of battery through pointers > >> > stored in power_supply structure. However, because the core owns the > >> > power supply instance, after calling power_supply_unregister() the > >> > driver cannot access these members. > >> > > >> > Fix this by using resource-managed allocations so internal data will be > >> > freed by pointers stored in resource-managed core. > >> > > >> > Signed-off-by: Krzysztof Kozlowski > >> > Reported-by: H.J. Lu > >> > Fixes: 297d716f6260 ("power_supply: Change ownership from driver to core") > >> > Cc: > >> > >> Applied to for-4.2/upstream-fixes, thanks. > > > > Wait, what guarantees do we have that this is only called in probe() > > paths? Don't we allow hid_hw_start() be deferred to open() calls? > > Indeed, this may be called in other contexts. But this should not > introduce errors except not reclaimable memory (till remove() > happens). > > > In general we need to be careful with devm* conversions in core code. > > > > Another and less intrusive fix would be: > > char *name = dev->battery->desc->name; > struct power_supply_desc *psy_desc = dev->battery->desc; > power_supply_unregister(dev->battery); > kfree(name); > kfree(psy_desc); I would much rather prefer this to the other version as it does not leave memory hanging around, potentially indefinitely, but ultimately it is up to Jiri. I only hope that power supply code does not reference power_supply_desc pointer past unregister (since the device structure itself may live past the point where power_supply_unregister() returns). By the way, you do not need name temp, you can do kfree(psy_desc->name); kfree(psy_desc); Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/