Re: [PATCH 15/30] staging: wilc1000: use kmemdup instead of kmalloc in add_network_to_shadow()

[Ajay Singh <ajay.kathat@microchip.com>]

On Thu, 10 May 2018 08:35:29 +0300
Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:

> On 09.05.2018 22:17, Ajay Singh wrote:
> > On Wed, 9 May 2018 16:42:59 +0300
> > Claudiu Beznea <Claudiu.Beznea@microchip.com> 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 <ajay.kathat@microchip.com>
> >>> ---
> >>>  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
> > 
> >