From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755162AbbGTU6B (ORCPT ); Mon, 20 Jul 2015 16:58:01 -0400 Received: from mout.gmx.net ([212.227.15.19]:63952 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752319AbbGTU57 (ORCPT ); Mon, 20 Jul 2015 16:57:59 -0400 Date: Mon, 20 Jul 2015 22:57:53 +0200 From: Christian Engelmayer To: Gwendal Grignou Cc: Olof Johansson , Lee Jones , Bill Richardson , Stephen Barber , Linux Kernel Subject: Re: [PATCH] mfd: cros_ec: Fix possible leak in led_rgb_store() Message-ID: <20150720225753.3eb21ef8@viper> In-Reply-To: References: <1437334982-3388-1-git-send-email-cengelma@gmx.at> X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:DTUFJ9FZZquY/tOBxEywcMVML1N8an5ra/uI9mn9jpJdMsfxwv+ lJ+r9Z3du3NBkEm4jxLQ4xr5cnFZXQX6cMwcKzkLIAOlIFD7l4B5nuYIB3zrH7H+XfWj8OM d3+t1mIFF/ZjiQnGAL1n6wi9C5DDI3gEcUy7TP+mqr6voUowlSLP1QuJORDpdvdRbX4yu9G pQckNNyv18mkYUgKKeGoQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:KceiHWYlKV4=:1qbViMhczVY9RfPUVqBMdE eKAQ0byKHe+yD37nUpuVD1Q5AYSwy80eawpSZAp2ODKOayOrlFOqhCk0vkMy8CENly8ubedwB 5CLYpfv2Q3ipRvckq95egwBMsi2/5ElDZttvK4Z8vZdMybukEiyzBobgNwnOYrruryUbnXqH7 h2Q6PfGL86l1vwCYs6RC4CVJeJKll9V28tmCeskZitmaUdyxhAN7BmhHhKN2rKCyvvVKJj0T7 5CnBI+tC6BIVpiqFnMNPjrUbRZLSBVUKAMDrn9jfsyC4GtLA7TiWnH0+GpelUleVdVdQyO5qd 0WZFTNd6Xbjp5KehQHutXRSVgbflTxBBP1ahgmHdKhwWHiroEIgqfUmdxsVSp/Bxc9JSq/I5i bk8k+9V4uXb1ii7//MKBTvXIFfSkn3IPN4zeKtgRPmANXr1jJC25kyncNCzKcV/gR2gw4J8U/ vG5Wy2ENyBNTsyx+OBxQosG8DQGy4ZV7BP5zuIH+Y5Ig03nl4KI9pQ9/yRcwfGM8Qo5AD0fOu 4433VpLlVGY+pBq6g1XEt/snDtxXQiZ8tRomMSoDMLrAbHA4YklU3KZCTrvvYNZMWEXVMIHM1 WnbV0nqUaMqb5pxXh5UWnIFce1FbqCAftC0TsrOFfvm4bkISrDooP/XMnAaaAOGdlLk9UXP+k xfbWYVKEL6wFur4EzF+Mrzx5bpOfAqtX1MW0jnVgVTWLNbaWTq5AqQ6JjWLpUcvrfa2g= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 20 Jul 2015 07:50:36 -0700, Gwendal Grignou wrote: > On Sun, Jul 19, 2015 at 12:43 PM, Christian Engelmayer wrote: > > Function led_rgb_store() contains some direct returns in error cases that > > leak the already allocated cros_ec_command message structure. Make sure > > that 'msg' is freed in all exit paths. Detected by Coverity CID 1309666. > > > > Signed-off-by: Christian Engelmayer > > --- > > Compile tested only. Applies against linux-next. > > --- > > drivers/platform/chrome/cros_ec_lightbar.c | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c > > index 144e09df9b84..4e598c11e8a4 100644 > > --- a/drivers/platform/chrome/cros_ec_lightbar.c > > +++ b/drivers/platform/chrome/cros_ec_lightbar.c > > @@ -252,7 +252,7 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, > > > > ret = sscanf(buf, "%i", &val[i++]); > > if (ret == 0) > > - return -EINVAL; > > + goto exit; > > > > if (i == 4) { > > param = (struct ec_params_lightbar *)msg->data; > > @@ -268,17 +268,15 @@ static ssize_t led_rgb_store(struct device *dev, struct device_attribute *attr, > > if ((j++ % 4) == 0) { > > ret = lb_throttle(); > > if (ret) > > - return ret; > > + goto exit; > > } > > > > ret = cros_ec_cmd_xfer(ec->ec_dev, msg); > > if (ret < 0) > > goto exit; > > > > - if (msg->result != EC_RES_SUCCESS) { > > - ret = -EINVAL; > ret = -EINVAL is necessary to indicate the command did not succeed: > the command was successfully sent to the EC, and the response was > received, but the EC failed the command internally. That's the code pattern seen in this module, however, in that case setting 'ret' seems superfluous and potentially misleading, as the functions exit code is written differently: exit: kfree(msg); return (ok && i == 0) ? count : -EINVAL; > > + if (msg->result != EC_RES_SUCCESS) > > goto exit; > > - } > > > > i = 0; > > ok = 1; > > -- > > 1.9.1 > >