| Message ID | 1525682614-3824-16-git-send-email-ajay.kathat@microchip.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Kalle Valo |
| Headers | show |
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 ? > shadow_nw_info->time_scan = jiffies; > shadow_nw_info->time_scan_cached = jiffies; > shadow_nw_info->found = 1; >
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. > > > shadow_nw_info->time_scan = jiffies; > > shadow_nw_info->time_scan_cached = jiffies; > > shadow_nw_info->found = 1; > > Regards, Ajay
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; } > > >> >>> shadow_nw_info->time_scan = jiffies; >>> shadow_nw_info->time_scan_cached = jiffies; >>> shadow_nw_info->found = 1; >>> > > > Regards, > Ajay > >
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 > > > >
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); shadow_nw_info->time_scan = jiffies; shadow_nw_info->time_scan_cached = jiffies; shadow_nw_info->found = 1;
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(-)