From: Paul Bolle <pebolle@tiscali•nl>
To: SF Markus Elfring <elfring@users•sourceforge.net>
Cc: gigaset307x-common@lists•sourceforge.net, netdev@vger•kernel.org,
Karsten Keil <isdn@linux-pingi•de>,
LKML <linux-kernel@vger•kernel.org>,
kernel-janitors@vger•kernel.org,
Julia Lawall <julia.lawall@lip6•fr>
Subject: Re: [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure
Date: Mon, 26 Sep 2016 23:13:54 +0200 [thread overview]
Message-ID: <1474924434.8546.29.camel@tiscali.nl> (raw)
In-Reply-To: <6409b3b0-3b7a-1139-f040-7b1a61611187@users.sourceforge.net>
Markus,
On Mon, 2016-09-26 at 17:43 +0200, SF Markus Elfring wrote:
> Memory was not released (as it would be expected) when one call
> of further resource reservations failed.
This was the only thing in this series that triggered more than a, very
uninspired, "meh" on first read.
> * 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
A quick scan of that link suggests we can do without the above
"background information" in the commit explanation.
> * Adjust jump targets according to the Linux coding style convention.
Another "meh".
> Signed-off-by: Markus Elfring <elfring@users•sourceforge.net>
> ---
> drivers/isdn/gigaset/common.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/isdn/gigaset/common.c
> b/drivers/isdn/gigaset/common.c
> index c05a2a4..2e9382f 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -710,10 +710,13 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
> cs->mode = M_UNKNOWN;
> cs->mstate = MS_UNINITIALIZED;
> cs->bcs = kmalloc_array(channels, sizeof(*cs->bcs), GFP_KERNEL);
> + if (!cs->bcs)
> + goto report_failure;
> +
> cs->inbuf = kmalloc(sizeof(*cs->inbuf), GFP_KERNEL);
> - if (!cs->bcs || !cs->inbuf) {
> - goto error;
> - }
> + if (!cs->inbuf)
> + goto free_bcs;
> +
> ++cs->cs_init;
>
> gig_dbg(DEBUG_INIT, "setting up at_state");
> @@ -737,14 +740,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
> gig_dbg(DEBUG_INIT, "setting up iif");
> if (gigaset_isdn_regdev(cs, modulename) < 0) {
> pr_err("error registering ISDN device\n");
> - goto error;
> + goto free_bcs;
> }
>
> make_valid(cs, VALID_ID);
> ++cs->cs_init;
> gig_dbg(DEBUG_INIT, "setting up hw");
> if (cs->ops->initcshw(cs) < 0)
> - goto error;
> + goto free_bcs;
>
> ++cs->cs_init;
>
> @@ -759,7 +762,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
> gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
> if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
> pr_err("could not allocate channel %d data\n", i);
> - goto error;
> + goto free_bcs;
> }
> }
>
> @@ -772,8 +775,9 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>
> gig_dbg(DEBUG_INIT, "cs initialized");
> return cs;
> -
> -error:
> +free_bcs:
> + kfree(cs->bcs);
> +report_failure:
> gig_dbg(DEBUG_INIT, "failed");
> gigaset_freecs(cs);
gigaset_freecs() is not a function I look at for the fun of it. But
still, in it we find:
case 0: /* error in basic setup */
[...]
kfree(cs->inbuf);
kfree(cs->bcs);
As far as I can tell we will call those two kfree()'s if we jump to
"error". So, contrary to your analysis, I don't think we leak cs->bcs.
> return NULL;
Paul Bolle
next prev parent reply other threads:[~2016-09-26 21:13 UTC|newest]
Thread overview: 192+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <566ABCD9.1060404@users.sourceforge.net>
2015-12-31 21:47 ` [PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection SF Markus Elfring
2016-01-07 11:07 ` Robert Richter
2016-01-07 19:30 ` SF Markus Elfring
2016-01-07 19:44 ` Joe Perches
2016-01-07 19:56 ` SF Markus Elfring
2016-01-07 19:59 ` Joe Perches
2016-01-07 20:07 ` SF Markus Elfring
2016-01-07 20:28 ` Joe Perches
2016-01-07 20:38 ` SF Markus Elfring
2016-01-07 20:42 ` Joe Perches
2015-12-31 23:22 ` [PATCH] be2net: Delete an unnecessary check in two functions SF Markus Elfring
2016-01-06 6:25 ` Sathya Perla
2016-01-01 12:18 ` [PATCH 0/3] net-gianfar: Fine-tuning for gfar_ethflow_to_filer_table() SF Markus Elfring
2016-01-01 12:22 ` [PATCH 1/3] net-gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection SF Markus Elfring
2016-01-01 12:35 ` Julia Lawall
2016-01-01 12:50 ` SF Markus Elfring
2016-01-01 13:05 ` Julia Lawall
2016-01-01 14:45 ` Francois Romieu
2016-01-01 13:04 ` [PATCH v2 " SF Markus Elfring
2016-01-02 3:16 ` David Miller
2016-01-01 12:23 ` [PATCH 2/3] net-gianfar: Delete unnecessary variable initialisations in gfar_ethflow_to_filer_table() SF Markus Elfring
2016-01-01 12:24 ` [PATCH 3/3] net-gianfar: Extend an initialisation clause of a for loop " SF Markus Elfring
2016-01-15 10:09 ` [PATCH v3 0/3] gianfar: Fine-tuning for gfar_ethflow_to_filer_table() SF Markus Elfring
2016-01-15 10:11 ` [PATCH v3 1/3] gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection SF Markus Elfring
2016-01-15 10:37 ` Joe Perches
2016-01-15 11:47 ` SF Markus Elfring
2016-01-15 12:03 ` Joe Perches
2016-01-15 17:32 ` SF Markus Elfring
2016-01-18 13:11 ` Claudiu Manoil
2016-01-15 10:12 ` [PATCH v3 2/3] gianfar: Delete unnecessary variable initialisations in gfar_ethflow_to_filer_table() SF Markus Elfring
2016-01-15 10:29 ` Dan Carpenter
2016-01-15 11:34 ` SF Markus Elfring
2016-01-15 12:15 ` Dan Carpenter
2016-01-15 16:42 ` David Miller
2016-01-15 17:15 ` SF Markus Elfring
2016-01-15 10:14 ` [PATCH v3 3/3] gianfar: Extend an initialisation clause of a for loop " SF Markus Elfring
2016-01-01 14:32 ` [PATCH] net-i40e: Replace variable initialisations by assignments in i40e_vc_get_vf_resources_msg() SF Markus Elfring
2016-01-01 14:51 ` net-i40e: Reconsider further usage of variable "i" " SF Markus Elfring
2016-01-08 20:51 ` Nelson, Shannon
2016-01-07 22:43 ` [PATCH] net-i40e: Replace variable initialisations by assignments " Nelson, Shannon
2016-01-08 10:42 ` Jeff Kirsher
2016-01-01 15:57 ` [PATCH] net-huawei_cdc_ncm: Delete an unnecessary variable initialisation in huawei_cdc_ncm_bind() SF Markus Elfring
[not found] ` <566ABCD9.1060404-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-01 16:50 ` [PATCH 0/2] net-qmi_wwan: Fine-tuning for two function implementations SF Markus Elfring
[not found] ` <5686AE52.1020008-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-01 16:54 ` [PATCH 1/2] net-qmi_wwan: Refactoring for qmi_wwan_bind() SF Markus Elfring
2016-01-02 21:38 ` Bjørn Mork
2016-01-01 16:56 ` [PATCH 2/2] net-qmi_wwan: Delete an unnecessary variable initialisation in qmi_wwan_register_subdriver() SF Markus Elfring
2016-01-02 21:30 ` Bjørn Mork
[not found] ` <87ziwnpt2v.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-01-03 1:45 ` David Miller
2016-01-01 18:21 ` [PATCH 0/2] net-ath9k_htc: Fine-tuning for two function implementations SF Markus Elfring
2016-01-01 18:23 ` [PATCH 1/2] net-ath9k_htc: Delete an unnecessary variable initialisation in ath9k_hif_usb_rx_stream() SF Markus Elfring
[not found] ` <5686C42A.5000405-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-01 19:14 ` Oleksij Rempel
2016-01-01 18:25 ` [PATCH 2/2] net-ath9k_htc: Replace a variable initialisation by an assignment in ath9k_htc_set_channel() SF Markus Elfring
2016-01-01 19:14 ` Oleksij Rempel
2016-04-08 1:40 ` Julian Calaby
2016-04-15 12:09 ` Kalle Valo
2016-04-15 14:34 ` Julian Calaby
2016-04-19 16:13 ` Kalle Valo
2016-01-01 19:26 ` [PATCH] net-brcmfmac: Delete an unnecessary variable initialisation in brcmf_sdio_download_firmware() SF Markus Elfring
[not found] ` <5686D2E0.2010309-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-02 8:50 ` Arend van Spriel
2016-01-14 6:58 ` Kalle Valo
2016-01-01 20:27 ` [PATCH 0/3] net-iwlegacy: Fine-tuning for il_eeprom_init() SF Markus Elfring
2016-01-01 20:30 ` [PATCH 1/3] net-iwlegacy: Refactoring " SF Markus Elfring
[not found] ` <5686E1D2.60107-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-04 9:33 ` Stanislaw Gruszka
2016-01-01 20:31 ` [PATCH 2/3] net-iwlegacy: One check less in il_eeprom_init() after error detection SF Markus Elfring
2016-01-01 23:13 ` Sergei Shtylyov
2016-01-01 20:32 ` [PATCH 3/3] net-iwlegacy: Another refactoring for il_eeprom_init() SF Markus Elfring
2016-01-02 18:18 ` Souptick Joarder
2016-01-01 21:33 ` [PATCH] net-libertas: Better exception handling in if_spi_host_to_card_worker() SF Markus Elfring
2016-01-01 21:41 ` Julia Lawall
2016-01-01 23:14 ` Sergei Shtylyov
2016-01-02 8:09 ` SF Markus Elfring
[not found] ` <568785B3.5000905-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-02 8:21 ` Julia Lawall
2016-01-02 9:08 ` SF Markus Elfring
2016-01-02 10:13 ` Arend van Spriel
2016-01-02 11:21 ` SF Markus Elfring
2016-01-03 9:36 ` Arend van Spriel
[not found] ` <5688EBAC.5000701-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-01-03 12:13 ` SF Markus Elfring
2016-01-03 15:18 ` Rafał Miłecki
2016-01-04 10:05 ` Arend van Spriel
2016-01-04 11:18 ` Rafał Miłecki
2016-01-21 15:07 ` [PATCH] " Kalle Valo
2016-01-02 14:40 ` [PATCH 0/3] net-rsi: Fine-tuning for two function implementations SF Markus Elfring
2016-01-02 14:43 ` [PATCH 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt() SF Markus Elfring
2016-01-02 15:12 ` net-rsi: Reconsider usage of variable "vap_id" " SF Markus Elfring
2016-01-02 16:32 ` [PATCH 1/3] rsi: Delete unnecessary variable initialisations " kbuild test robot
2016-01-02 18:27 ` [PATCH v2 " SF Markus Elfring
2016-01-04 9:28 ` [PATCH " Dan Carpenter
2016-01-04 9:38 ` Dan Carpenter
2016-01-04 10:44 ` SF Markus Elfring
2016-01-04 11:48 ` Dan Carpenter
2016-01-04 12:33 ` SF Markus Elfring
[not found] ` <568A668D.8090007-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-04 23:54 ` Julian Calaby
2016-01-05 8:29 ` SF Markus Elfring
2016-01-05 9:47 ` Julian Calaby
2016-01-05 16:23 ` SF Markus Elfring
2016-01-04 13:17 ` [PATCH 1/3] " Bjørn Mork
[not found] ` <87poxhiivf.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
2016-01-04 14:25 ` Dan Carpenter
2016-01-04 17:14 ` David Miller
2016-01-02 14:44 ` [PATCH 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt() SF Markus Elfring
2016-01-02 14:45 ` [PATCH 3/3] rsi: Replace variable initialisations by assignments " SF Markus Elfring
2016-01-02 15:07 ` Francois Romieu
[not found] ` <5687E169.4070704-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-01-15 13:04 ` [PATCH v3 0/3] net-rsi: Fine-tuning for two function implementations SF Markus Elfring
2016-01-15 13:09 ` [PATCH v3 1/3] rsi: Delete unnecessary variable initialisations in rsi_send_mgmt_pkt() SF Markus Elfring
2016-01-15 13:10 ` [PATCH v3 2/3] rsi: Delete unnecessary variable initialisations in rsi_send_data_pkt() SF Markus Elfring
2016-01-15 13:12 ` [PATCH v3 3/3] rsi: Replace variable initialisations by assignments " SF Markus Elfring
2016-01-19 12:40 ` Dan Carpenter
2016-01-02 17:50 ` [PATCH 0/5] xen-netback: Fine-tuning for three function implementations SF Markus Elfring
2016-01-02 17:54 ` [PATCH 1/5] xen-netback: Delete an unnecessary assignment in connect_rings() SF Markus Elfring
2016-01-02 17:55 ` [PATCH 2/5] xen-netback: Delete an unnecessary goto statement " SF Markus Elfring
2016-01-02 17:57 ` [PATCH 3/5] xen-netback: Replace a variable initialisation by an assignment in read_xenbus_vif_flags() SF Markus Elfring
2016-01-02 17:58 ` [PATCH 4/5] xen-netback: Replace a variable initialisation by an assignment in xen_register_watchers() SF Markus Elfring
2016-01-02 18:00 ` [PATCH 5/5] xen-netback: Delete an unnecessary variable initialisation " SF Markus Elfring
2016-01-03 1:34 ` [PATCH 0/5] xen-netback: Fine-tuning for three function implementations Joe Perches
2016-01-04 9:40 ` Dan Carpenter
2016-01-04 11:08 ` Wei Liu
2016-08-20 6:01 ` [PATCH] mlx5/core: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
[not found] ` <ceb39933-438d-920d-f294-ea0ce32fd171-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-08-20 9:32 ` walter harms
2016-08-23 0:05 ` David Miller
2016-08-20 7:27 ` [PATCH 0/2] tun: Fine-tuning for update_filter() SF Markus Elfring
2016-08-20 7:34 ` [PATCH 1/2] tun: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
2016-08-20 11:47 ` Shmulik Ladkani
2016-08-22 1:43 ` Michael S. Tsirkin
2016-08-20 7:37 ` [PATCH 2/2] tun: Rename a jump label in update_filter() SF Markus Elfring
2016-08-22 1:41 ` Michael S. Tsirkin
2016-08-22 5:26 ` Mike Rapoport
2016-08-21 2:11 ` [PATCH 0/2] tun: Fine-tuning for update_filter() David Miller
2016-08-20 16:43 ` [PATCH 0/3] hostap: Fine-tuning for a few functions SF Markus Elfring
2016-08-20 16:45 ` [PATCH 1/3] hostap: Use memdup_user() rather than duplicating its implementation SF Markus Elfring
[not found] ` <efa66c16-7998-9ceb-0be6-d62dd249a2dc-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-09-26 14:19 ` [1/3] " Kalle Valo
2016-08-20 16:46 ` [PATCH 2/3] hostap: Delete an unnecessary jump label in prism2_ioctl_priv_hostapd() SF Markus Elfring
2016-08-21 1:45 ` Julian Calaby
2016-09-26 15:06 ` [2/3] " Kalle Valo
[not found] ` <20160926150656.213D961568@smtp.codeaurora.org>
[not found] ` <20160926150656.213D961568-4h6buKAYkuurB/BPivuO70B+6BGkLq7r@public.gmane.org>
2016-09-26 16:03 ` SF Markus Elfring
2016-09-26 18:01 ` Kalle Valo
2016-09-26 18:06 ` SF Markus Elfring
2016-09-26 18:18 ` Joe Perches
2016-09-26 18:37 ` Kalle Valo
2016-09-26 18:43 ` Joe Perches
2016-08-20 16:48 ` [PATCH 3/3] hostap: Delete unnecessary initialisations for the variable "ret" SF Markus Elfring
[not found] ` <f0cd7a82-e603-6a91-0afd-33bbd1658cab-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
2016-08-20 19:26 ` [PATCH 0/3] hostap: Fine-tuning for a few functions Arend van Spriel
2016-08-22 15:49 ` Kalle Valo
2016-08-22 16:18 ` Joe Perches
2016-08-22 18:17 ` [PATCH] checkpatch: See if modified files are marked obsolete in MAINTAINERS Joe Perches
2016-08-22 20:50 ` SF Markus Elfring
2016-08-22 20:56 ` Joe Perches
2016-08-23 7:26 ` SF Markus Elfring
2016-08-23 10:18 ` Julia Lawall
2016-09-26 15:37 ` [PATCH 0/5] ISDN-Gigaset: Fine-tuning for three function implementations SF Markus Elfring
2016-09-26 15:38 ` [PATCH 1/5] ISDN-Gigaset: Use kmalloc_array() in two functions SF Markus Elfring
2016-09-28 11:37 ` Paul Bolle
2016-09-28 16:38 ` SF Markus Elfring
2016-09-28 17:44 ` Paul Bolle
2016-09-26 15:40 ` [PATCH 2/5] ISDN-Gigaset: Improve another size determination in gigaset_initcs() SF Markus Elfring
2016-09-26 15:42 ` [PATCH 3/5] ISDN-Gigaset: Delete an error message for a failed memory allocation SF Markus Elfring
2016-09-27 10:57 ` Tilman Schmidt
2016-09-28 11:42 ` Paul Bolle
2016-09-26 15:43 ` [PATCH 4/5] ISDN-Gigaset: Release memory in gigaset_initcs() after an allocation failure SF Markus Elfring
2016-09-26 21:13 ` Paul Bolle [this message]
2016-09-27 5:20 ` SF Markus Elfring
2016-09-27 8:48 ` Paul Bolle
2016-09-27 9:34 ` SF Markus Elfring
2016-09-27 9:48 ` Paul Bolle
2016-09-27 10:26 ` [Gigaset307x-common] " Tilman Schmidt
2016-09-27 11:32 ` SF Markus Elfring
2016-09-27 12:08 ` Tilman Schmidt
2016-09-27 12:16 ` isdn
2016-09-27 12:52 ` SF Markus Elfring
2016-09-27 14:35 ` Tilman Schmidt
2016-09-27 7:30 ` [PATCH 4/5] " Dan Carpenter
2016-09-27 15:10 ` SF Markus Elfring
2016-09-27 7:12 ` Dan Carpenter
2016-09-27 7:28 ` SF Markus Elfring
2016-09-26 15:44 ` [PATCH 5/5] ISDN-Gigaset: Enclose two expressions for the sizeof operator by parentheses SF Markus Elfring
2016-09-26 16:00 ` David Laight
2016-09-26 16:23 ` Joe Perches
2016-09-26 16:45 ` SF Markus Elfring
2016-09-26 16:50 ` Julia Lawall
2016-09-26 17:44 ` [PATCH v2 " SF Markus Elfring
2016-09-26 18:31 ` Paul Bolle
2016-09-26 18:43 ` SF Markus Elfring
2016-09-27 0:10 ` David Miller
2016-09-27 5:32 ` SF Markus Elfring
2016-09-26 17:38 ` [PATCH " Sergei Shtylyov
2016-09-26 18:00 ` SF Markus Elfring
2016-09-26 18:35 ` Paul Bolle
2016-09-27 7:08 ` [PATCH 5/5] " Dan Carpenter
2016-09-27 7:25 ` Dan Carpenter
2016-09-26 20:38 ` [PATCH 0/5] ISDN-Gigaset: Fine-tuning for three function implementations Paul Bolle
2016-09-27 5:10 ` SF Markus Elfring
2016-09-28 11:56 ` [PATCH 0/5] " Paul Bolle
2016-09-28 16:50 ` SF Markus Elfring
2016-09-28 17:57 ` Paul Bolle
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1474924434.8546.29.camel@tiscali.nl \
--to=pebolle@tiscali$(echo .)nl \
--cc=elfring@users$(echo .)sourceforge.net \
--cc=gigaset307x-common@lists$(echo .)sourceforge.net \
--cc=isdn@linux-pingi$(echo .)de \
--cc=julia.lawall@lip6$(echo .)fr \
--cc=kernel-janitors@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox