lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Feb 2012 21:10:09 -0500 From: Sean MacLennan <seanm@...nm.ca> To: Jesper Juhl <jj@...osbits.net> Cc: devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org, Andrea Merello <andreamrl@...cali.it>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Larry Finger <Larry.Finger@...inger.net>, Mike McCormack <mikem@...g3k.org> Subject: Re: [PATCH] Staging, rtl8192e, softmac: remove redundant memset and fix mem leak On Mon, 13 Feb 2012 00:15:02 +0100 (CET) Jesper Juhl <jj@...osbits.net> wrote: > We also fail to kfree() the memory we allocated for 'network' if we > do not enter > > if (ieee->current_network.qos_data.supported == 1) { > > and the variable then goes out of scope. > > To fix that I simply moved the kfree() that was inside that 'if' > statement to instead be just after it. It then covers both the case > where we take the branch and when we don't. Nice catch! We know that the driver leaks memory if left running for a long time, this will help! I would recommend a small change: instead of moving the kfree() out of the loop, why not move the kzalloc into it? The qos_data.supported == 0 is the normal case (at least for me), so why not save an alloc? Something like this: diff --git a/drivers/staging/rtl8192e/rtllib_softmac.c b/drivers/staging/rtl8192e/rtllib_softmac.c index 1637f11..59b991f 100644 --- a/drivers/staging/rtl8192e/rtllib_softmac.c +++ b/drivers/staging/rtl8192e/rtllib_softmac.c @@ -2228,13 +2228,6 @@ inline int rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb, (ieee->iw_mode == IW_MODE_INFRA)) { errcode = assoc_parse(ieee, skb, &aid); if (0 == errcode) { - struct rtllib_network *network = - kzalloc(sizeof(struct rtllib_network), - GFP_ATOMIC); - - if (!network) - return 1; - memset(network, 0, sizeof(*network)); ieee->state = RTLLIB_LINKED; ieee->assoc_id = aid; ieee->softmac_stats.rx_ass_ok++; @@ -2242,6 +2235,13 @@ inline int rtllib_rx_assoc_resp(struct rtllib_device *ieee, struct sk_buff *skb, /* Let the register setting default with Legacy station */ assoc_resp = (struct rtllib_assoc_response_frame *)skb->data; if (ieee->current_network.qos_data.supported == 1) { + struct rtllib_network *network = + kzalloc(sizeof(struct rtllib_network), + GFP_ATOMIC); + + if (!network) + return 1; + if (rtllib_parse_info_param(ieee, assoc_resp->info_element, rx_stats->len - sizeof(*assoc_resp), network, rx_stats)) { Cheers, Sean -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists