From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752723AbdECP6f (ORCPT ); Wed, 3 May 2017 11:58:35 -0400 Received: from mx02-sz.bfs.de ([194.94.69.103]:59828 "EHLO mx02-sz.bfs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752514AbdECP63 (ORCPT ); Wed, 3 May 2017 11:58:29 -0400 Message-ID: <5909FE21.7050607@bfs.de> Date: Wed, 03 May 2017 17:58:25 +0200 From: walter harms Reply-To: wharms@bfs.de User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.1.16) Gecko/20101125 SUSE/3.0.11 Thunderbird/3.0.11 MIME-Version: 1.0 To: Colin King CC: Greg Kroah-Hartman , Wei-Ning Huang , Thierry Escande , Wei Yongjun , kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] firmware: Google VPD: fix error handling on allocation failures References: <20170503154924.1659-1-colin.king@canonical.com> In-Reply-To: <20170503154924.1659-1-colin.king@canonical.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 03.05.2017 17:49, schrieb Colin King: > From: Colin Ian King > > Fix two allocation failure checks. Firstly, ensure info is checked > for a failed allocation; this fixes a potential null pointer > dereference issue on info->key. Secondly, free info is info->key > fails to allocate to fix a memory leak. > > Detected by CoverityScan, CID#1430064 ("Resource Leak") > > Signed-off-by: Colin Ian King > --- > drivers/firmware/google/vpd.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c > index 3ce813110d5e..2eb15b1dabcc 100644 > --- a/drivers/firmware/google/vpd.c > +++ b/drivers/firmware/google/vpd.c > @@ -116,9 +116,13 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len, > return VPD_OK; > > info = kzalloc(sizeof(*info), GFP_KERNEL); > + if (!info) > + return -ENOMEM; > info->key = kzalloc(key_len + 1, GFP_KERNEL); > - if (!info->key) > + if (!info->key) { > + kfree(info); > return -ENOMEM; > + } > > memcpy(info->key, key, key_len); > the use of key_len+1 looks like preparing space for a string. so kstrdup() seems more fitting. If this is not a string there is also a kmemdup(). re, wh