From patchwork Sat Dec 10 20:53:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: SF Markus Elfring X-Patchwork-Id: 9469509 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 032C8607EE for ; Sat, 10 Dec 2016 20:53:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E861025EA6 for ; Sat, 10 Dec 2016 20:53:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DAE7B284E5; Sat, 10 Dec 2016 20:53:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 696B225EA6 for ; Sat, 10 Dec 2016 20:53:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752510AbcLJUxW (ORCPT ); Sat, 10 Dec 2016 15:53:22 -0500 Received: from mout.web.de ([212.227.15.4]:55813 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752087AbcLJUxV (ORCPT ); Sat, 10 Dec 2016 15:53:21 -0500 Received: from [192.168.1.2] ([77.181.156.58]) by smtp.web.de (mrweb002 [213.165.67.108]) with ESMTPSA (Nemesis) id 0MXHoz-1cAVaV2vuH-00WHAF; Sat, 10 Dec 2016 21:53:11 +0100 Subject: [PATCH 4/4] [media] bt8xx: Less function calls in dst_ca_ioctl() after error detection To: linux-media@vger.kernel.org, Alexey Khoroshilov , Hans Verkuil , Mauro Carvalho Chehab References: Cc: LKML , kernel-janitors@vger.kernel.org From: SF Markus Elfring Message-ID: Date: Sat, 10 Dec 2016 21:53:09 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: X-Provags-ID: V03:K0:GPoHpZyeraXQhtVrviDaCvjgtQ71727wyU237hs5i83mLYYg2R7 wgJuUNuc7zRxgOU2GV9njtGGJqInmP7LRm/AGb7ZyGDSnEuuzUoMTvEFfby/+cxeUHU30vM JReoaGN8BcCa7i71uVpZmqNbV5lZXMWrfIHTMIe6k5StSOd+/d3/TLEv3C3i2Ae/vKnbmSA wiB8YT/BYTEizh4YQjLEQ== X-UI-Out-Filterresults: notjunk:1; V01:K0:JMnVjJzxO7Y=:MoF0B2ar97dJdhP0bPi7Jj sVSoLVhXgW42khs7sSydzN72VxIeuYVruhv34Uw9Vv3O2LHjdwCMPV/cDmEFaiq9m27qG2Y5W scrhef4iJRugxj3kiCxxFuS39Rd6OdXpGD6Nb5MwvwvGjpgxiKGs5hApJ1HnrUlXvJ8hOYCW9 qYdeVLtoCFx1gQgAhgEEg6XuNWcRCHCT+901U3LirJF4PXZncCRQgKSmtR/zpgFouRjcKhHjB vTpMwR1wCHWlQoCtYWIKdKhub4ubEzkJcWdMPMPu8/uQo+qGR8bv9v2lTsoqQPGTe+bueYz/Z yUKsEHZqYtrQY+9rkwyYIxO1mpNdsn+o+vU1EAol+blqh4OwgbA21ADXGLARjg+QLcbqJB2Xs A1uwdsdLalKcpJm/GvlkvDq94Ns3vsZCP/U6sHJgD29rX0mvLHdTaaHd5Q72vI5ve1oeIVzbK a6tAw6TPs2YGStLFs+vMPEY3xvVRqIPQ3cC1vzhS1l1ER6xu4JHuEey+EzeoEAVT5QcWbxJxb WgF7+NrBrQx3v5t0v14ryPUyIpBET14Sh8WXFEpRuHYf3CzJtJBshmlNwT5dwOpefoiFauC/d xlBRZr7oLLAXBNvfWsOI1vz+odckf6168c/UMIV3eRP7yO6z9Iyiebo7mDeHoCtgMPrSS5gCh fnRuXukoWJx+H8KyWwljcP9uYWVzKl8ctmAsH69Ysgb2cbFGz0TBYdHf/lplvlwT0EHajxFwe v0TNDAIE6R/Z5L9UAjiQNN4Wm6sGLTdwOpFgh/s93mowabc9F6PTrjzXjYhY8rRpsi9SysDon +zhHL27 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Markus Elfring Date: Sat, 10 Dec 2016 21:30:10 +0100 The kfree() function was called in up to three cases by the dst_ca_ioctl() function during error handling even if the passed variable contained a null pointer. This issue was detected by using the Coccinelle software. * Split a condition check for memory allocation failures so that each pointer from these function calls will be checked immediately. See also background information: Topic "CWE-754: Improper check for unusual or exceptional conditions" Link: https://cwe.mitre.org/data/definitions/754.html Fixes: b57e5578f913a304e97cb66aa0044a894ca47f2f ("Fixes some sync issues between V4L/DVB development and GIT") * Replace the specification of data structures by pointer dereferences to make the corresponding size determination a bit safer. * Adjust jump targets according to the Linux coding style convention. Signed-off-by: Markus Elfring --- drivers/media/pci/bt8xx/dst_ca.c | 51 +++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c index 04d06c564602..50cdb53c9e8a 100644 --- a/drivers/media/pci/bt8xx/dst_ca.c +++ b/drivers/media/pci/bt8xx/dst_ca.c @@ -559,16 +559,27 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct int result = 0; mutex_lock(&dst_ca_mutex); - dvbdev = file->private_data; - state = (struct dst_state *)dvbdev->priv; - p_ca_message = kmalloc(sizeof (struct ca_msg), GFP_KERNEL); - p_ca_slot_info = kmalloc(sizeof (struct ca_slot_info), GFP_KERNEL); - p_ca_caps = kmalloc(sizeof (struct ca_caps), GFP_KERNEL); - if (!p_ca_message || !p_ca_slot_info || !p_ca_caps) { + p_ca_message = kmalloc(sizeof(*p_ca_message), GFP_KERNEL); + if (!p_ca_message) { result = -ENOMEM; - goto free_mem_and_exit; + goto unlock; + } + + p_ca_slot_info = kmalloc(sizeof(*p_ca_slot_info), GFP_KERNEL); + if (!p_ca_slot_info) { + result = -ENOMEM; + goto free_message; } + p_ca_caps = kmalloc(sizeof(*p_ca_caps), GFP_KERNEL); + if (!p_ca_caps) { + result = -ENOMEM; + goto free_slot_info; + } + + dvbdev = file->private_data; + state = (struct dst_state *)dvbdev->priv; + /* We have now only the standard ioctl's, the driver is upposed to handle internals. */ switch (cmd) { case CA_SEND_MSG: @@ -576,7 +587,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_send_message(state, p_ca_message, arg)) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_SEND_MSG Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } break; case CA_GET_MSG: @@ -584,7 +595,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_get_message(state, p_ca_message, arg)) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_GET_MSG Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_GET_MSG Success !"); break; @@ -598,7 +609,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_get_slot_info(state, p_ca_slot_info, arg)) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_GET_SLOT_INFO Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_GET_SLOT_INFO Success !"); break; @@ -607,7 +618,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_get_slot_caps(state, p_ca_caps, arg)) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_GET_CAP Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_GET_CAP Success !"); break; @@ -616,7 +627,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_get_slot_descr(state, p_ca_message, arg)) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_GET_DESCR_INFO Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_GET_DESCR_INFO Success !"); break; @@ -625,7 +636,7 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_set_slot_descr()) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_SET_DESCR Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_SET_DESCR Success !"); break; @@ -634,17 +645,19 @@ static long dst_ca_ioctl(struct file *file, unsigned int cmd, unsigned long ioct if ((ca_set_pid()) < 0) { dprintk(verbose, DST_CA_ERROR, 1, " -->CA_SET_PID Failed !"); result = -1; - goto free_mem_and_exit; + goto free_caps; } dprintk(verbose, DST_CA_INFO, 1, " -->CA_SET_PID Success !"); default: result = -EOPNOTSUPP; } - free_mem_and_exit: - kfree (p_ca_message); - kfree (p_ca_slot_info); - kfree (p_ca_caps); - +free_caps: + kfree(p_ca_caps); +free_slot_info: + kfree(p_ca_slot_info); +free_message: + kfree(p_ca_message); +unlock: mutex_unlock(&dst_ca_mutex); return result; }