From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759739AbbBIH4b (ORCPT ); Mon, 9 Feb 2015 02:56:31 -0500 Received: from mailout4.w1.samsung.com ([210.118.77.14]:24992 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759722AbbBIH42 (ORCPT ); Mon, 9 Feb 2015 02:56:28 -0500 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfec7f5-b7fc86d0000066b7-9e-54d86796a936 Content-transfer-encoding: 8BIT Message-id: <1423468583.4909.4.camel@AMDC1943> Subject: Re: [RFT PATCH 2/4] compal-laptop: Check return value of power_supply_register From: Krzysztof Kozlowski To: Darren Hart Cc: Dmitry Artamonow , Marek Belisko , Cezary Jackiewicz , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, stable@vger.kernel.org Date: Mon, 09 Feb 2015 08:56:23 +0100 In-reply-to: <20150207024245.GB36295@fury.dvhart.com> References: <1422358221-13199-1-git-send-email-k.kozlowski@samsung.com> <1422358221-13199-3-git-send-email-k.kozlowski@samsung.com> <20150207024245.GB36295@fury.dvhart.com> X-Mailer: Evolution 3.10.4-0ubuntu2 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrELMWRmVeSWpSXmKPExsVy+t/xq7rT0m+EGPx/I2Wx5EQ3i8WkJ++Z LboWGlhMXDmZ2eLyrjlsFp97jzBabG87ymLxb+kWNovVe14wW5zeXWKxYOMjRgduj52z7rJ7 rHl/itnj2LU7LB6bV2h5bFrVyebxeZNcAFsUl01Kak5mWWqRvl0CV8aZEzNZCyaIVFz8uo+9 gXGtQBcjJ4eEgInEl9ZHbBC2mMSFe+uBbC4OIYGljBK7Pt5kBEnwCghK/Jh8j6WLkYODWUBe 4silbJAws4C6xKR5i5gh6j8zSuw48QSshldAT+L+VX6QGmGBSImj0xvB5rMJGEtsXr4EzBYR UJPYt+QuC0gvs8APJol7m56ygiRYBFQldsx6ALaXU8BUonvLRBaIBRsYJU4/ncIOskBCQFmi sd9tAqPALCTnzUI4bxaS8xYwMq9iFE0tTS4oTkrPNdIrTswtLs1L10vOz93ECImHrzsYlx6z OsQowMGoxMNroXw9RIg1say4MvcQowQHs5IIr9FHoBBvSmJlVWpRfnxRaU5q8SFGJg5OqQZG PbPvIpfXfbi2p3uhbWfdNIMZZu8/2K7NDYkqe2L6eo30imgL5zi7xmw1pv5867+xynnLbrkL iT5inPqsUHjWkq8rJE68O3pk99O6NE/7NivX03Pnvt7+9sOGfyWRsYe/3uANb6oOEf33qE33 4UW9qz/KTz+7v7mE55hp7mPRsx+P+7EbNZ9lVGIpzkg01GIuKk4EAOmk7rVlAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On pią, 2015-02-06 at 18:42 -0800, Darren Hart wrote: > On Tue, Jan 27, 2015 at 12:30:19PM +0100, Krzysztof Kozlowski wrote: > > The return value of power_supply_register() call was not checked and > > even on error probe() function returned 0. If registering failed then > > during unbind the driver tried to unregister power supply which was not > > actually registered. > > > > This could lead to memory corruption because power_supply_unregister() > > unconditionally cleans up given power supply. > > > > Fix this by checking return status of power_supply_register() call. In > > case of failure, unregister the hwmon device and fail the probe. Add a > > fixme note about missing hwmon_device_unregister() in driver removal. > > > > Signed-off-by: Krzysztof Kozlowski > > Fixes: 9be0fcb5ed46 ("compal-laptop: add JHL90, battery & hwmon interface") > > Cc: > > --- > > drivers/platform/x86/compal-laptop.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/x86/compal-laptop.c b/drivers/platform/x86/compal-laptop.c > > index 15c0fab2bfa1..cf55a9246f12 100644 > > --- a/drivers/platform/x86/compal-laptop.c > > +++ b/drivers/platform/x86/compal-laptop.c > > @@ -1036,12 +1036,16 @@ static int compal_probe(struct platform_device *pdev) > > > > /* Power supply */ > > initialize_power_supply_data(data); > > - power_supply_register(&compal_device->dev, &data->psy); > > + err = power_supply_register(&compal_device->dev, &data->psy); > > + if (err < 0) > > + goto psy_err; > > > > platform_set_drvdata(pdev, data); > > > > return 0; > > > > +psy_err: > > + hwmon_device_unregister(hwmon_dev); > > remove: > > sysfs_remove_group(&pdev->dev.kobj, &compal_platform_attr_group); > > return err; > > @@ -1072,6 +1076,7 @@ static int compal_remove(struct platform_device *pdev) > > > > data = platform_get_drvdata(pdev); > > power_supply_unregister(&data->psy); > > + /* FIXME: missing hwmon_device_unregister() */ > > Is this FIXME a leftover? Is there a reason we can't fix this now instead of > adding a FIXME? This is not a leftover. I think this should be fixed but: 1. I cannot test this driver, 2. I am not such familiar with hwmon API, so my fix could be wrong and introduce more errors than fixes. If you also think hwmon_device_unregister() is needed then I could send new version of patch. Also I would be happy if someone else fixed this. Best regards, Krzysztof