From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755581AbbBGCnB (ORCPT ); Fri, 6 Feb 2015 21:43:01 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:44066 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751412AbbBGCm7 (ORCPT ); Fri, 6 Feb 2015 21:42:59 -0500 Date: Fri, 6 Feb 2015 18:42:45 -0800 From: Darren Hart To: Krzysztof Kozlowski 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 Subject: Re: [RFT PATCH 2/4] compal-laptop: Check return value of power_supply_register Message-ID: <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422358221-13199-3-git-send-email-k.kozlowski@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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? -- Darren Hart Intel Open Source Technology Center