From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 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> Sender: linux-kernel-owner@vger.kernel.org List-ID: 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