Inconsistent use of size argument in kzalloc and memcpy in 'drivers/net/ethernet/toshiba/ps3_gelic_wireless.c'
Subsystems (4)
Hi,
while looking at potential clean-up, I ended on the following code which looks spurious to me.
We allocate 'be16_to_cpu(scan_info->size)' bytes, but then copy 'scan_info->size'.
This is not consistent.
I don't know which one is the correct one.
CJ
tmp/cocci-output-24201-0dddbd-ps3_gelic_wireless.c renamed from net/ethernet/toshiba/ps3_gelic_wireless.c +0 -3
@@ -1616,13 +1616,10 @@ static void gelic_wl_scan_complete_event target->valid =1 ; target->eurus_index = i;kfree (target->hwinfo); - target->hwinfo =kzalloc (be16_to_cpu (scan_info->size), -GFP_KERNEL );if (!target->hwinfo)continue ;/* copy hw scan info */ -memcpy (target->hwinfo, scan_info, scan_info->size); target->essid_len =strnlen (scan_info->essid,sizeof (scan_info->essid)); target->rate_len =0 ;
this is a case for kmemdup().
target->hwinfo=kmemdup(scan_info,be16_to_cpu(scan_info->size), GFP_KERNEL);
re,
wh
Am 11.04.2016 12:00, schrieb Christophe JAILLET:
Quoted 38 lines
Hi,
while looking at potential clean-up, I ended on the following code which looks spurious to me.
We allocate '
be16_to_cpu(scan_info->size)' bytes, but then copy 'scan_info->size'.
This is not consistent.I don't know which one is the correct one.
CJ
tmp/cocci-output-24201-0dddbd-ps3_gelic_wireless.c renamed from net/ethernet/toshiba/ps3_gelic_wireless.c +0
-3@@ -1616,13 +1616,10 @@ static void gelic_wl_scan_complete_event target->valid =1 ; target->eurus_index = i;kfree (target->hwinfo); - target->hwinfo =kzalloc (be16_to_cpu (scan_info->size), -GFP_KERNEL );if (!target->hwinfo)continue ;/* copy hw scan info */ -memcpy (target->hwinfo, scan_info, scan_info->size); target->essid_len =strnlen (scan_info->essid,sizeof (scan_info->essid)); target->rate_len =0 ;Signature · 5 lines
-- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Christophe JAILLET
Hi,
while looking at potential clean-up, I ended on the following code which looks spurious to me.
We allocate '
be16_to_cpu(scan_info->size)' bytes, but then copy 'scan_info->size'.
This is not consistent.
Good catch. be16_to_cpu(scan_info->size) is correct. It's surprising that this bug wasn't caught in testing...
regards,
dan carpenter