Return-path: Received: from esa5.microchip.iphmx.com ([216.71.150.166]:23853 "EHLO esa5.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934243AbeEJHrx (ORCPT ); Thu, 10 May 2018 03:47:53 -0400 Date: Thu, 10 May 2018 13:17:45 +0530 From: Ajay Singh To: Claudiu Beznea CC: , , , , , , Subject: Re: [PATCH 15/30] staging: wilc1000: use kmemdup instead of kmalloc in add_network_to_shadow() Message-ID: <20180510131745.2e40a16a@ajaysk-VirtualBox> (sfid-20180510_094757_100078_5FFDA284) In-Reply-To: References: <1525682614-3824-1-git-send-email-ajay.kathat@microchip.com> <1525682614-3824-16-git-send-email-ajay.kathat@microchip.com> <20180510004731.64cd2d32@ajaysk-VirtualBox> MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Sender: linux-wireless-owner@vger.kernel.org List-ID: On Thu, 10 May 2018 08:35:29 +0300 Claudiu Beznea wrote: > On 09.05.2018 22:17, Ajay Singh wrote: > > On Wed, 9 May 2018 16:42:59 +0300 > > Claudiu Beznea wrote: > > > >> On 07.05.2018 11:43, Ajay Singh wrote: > >>> Use kmemdup instead of kmalloc & memcpy in > >>> add_network_to_shadow(). > >>> > >>> Signed-off-by: Ajay Singh > >>> --- > >>> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c > >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c index > >>> 0ae2065..ca221f1 100644 --- > >>> a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c +++ > >>> b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c @@ -331,8 > >>> +331,8 @@ static void add_network_to_shadow(struct network_info > >>> *nw_info, shadow_nw_info->tsf_hi = nw_info->tsf_hi; if > >>> (ap_found != -1) kfree(shadow_nw_info->ies); > >>> - shadow_nw_info->ies = kmalloc(nw_info->ies_len, > >>> GFP_KERNEL); > >>> - memcpy(shadow_nw_info->ies, nw_info->ies, > >>> nw_info->ies_len); > >>> + shadow_nw_info->ies = kmemdup(nw_info->ies, > >>> nw_info->ies_len, > >>> + GFP_KERNEL); > >> > >> Maybe, in case of NULL, you will want to set ies_len = 0 ? > > > > > > I couldn't find code where 'ies_len' is check to validity of data. > > Mostly we use NULL check for "ies" pointer for data > > validity.So in my opinion setting it to zero would be > > irrelevant. > > I'm seeing this in refresh_scan(): > network_info = > &last_scanned_shadow[i]; > if (!memcmp("DIRECT-", network_info->ssid, 7) > && !direct_scan) > continue; > freq = > ieee80211_channel_to_frequency((s32)network_info->ch, > NL80211_BAND_2GHZ); channel = ieee80211_get_channel(wiphy, > freq); rssi = > get_rssi_avg(network_info); bss = > cfg80211_inform_bss(wiphy, channel, > CFG80211_BSS_FTYPE_UNKNOWN, > network_info->bssid, > network_info->tsf_hi, > network_info->cap_info, > network_info->beacon_period, > (const u8 > *)network_info->ies, (size_t)network_info->ies_len, > (s32)rssi * > 100, GFP_KERNEL); > > Looking further into cfg80211_inform_bss(): > -> cfg80211_inform_bss_data() > -> cfg80211_get_bss_channel() > -> cfg80211_find_ie() > -> cfg80211_find_ie_match(): > while (len >= 2 && len >= ies[1] + 2) > { if ((ies[0] == eid) && > (ies[1] + 2 >= match_offset + match_len) > && !memcmp(ies + match_offset, match, match_len)) > return > ies; > len -= ies[1] + > 2; ies += ies[1] + 2; > } > > Got it. I will also include the code to set ies_len to 0 during memory allocations failure scenario. > > > > > >> > >>> shadow_nw_info->time_scan = jiffies; > >>> shadow_nw_info->time_scan_cached = jiffies; > >>> shadow_nw_info->found = 1; > >>> > > > > > > Regards, > > Ajay > > > >