Re: memcpy overlap in getinterfaces(int*) (tcpip.cc:2987)

[David Fifield <david () bamsoftware com>]

On Tue, Jun 16, 2009 at 10:07:23PM +0000, Brandon Enright wrote:
> I was recently testing Nmap in Valgrind and noticed that I always get
> the following warning:
>
> ==20569== Source and destination overlap in memcpy(0x7FEFFA160, 0x7FEFFA160, 16)
> ==20569==    at 0x4C2590A: memcpy (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
> ==20569==    by 0x453AB7: getinterfaces(int*) (tcpip.cc:2987)
> ==20569==    by 0x454ADD: route_dst(sockaddr_storage const*, route_nfo*) (tcpip.cc:3376)
> ==20569==    by 0x452201: nexthost(HostGroupState*, TargetGroup*, scan_lists*, int) (targets.cc:492)
> ==20569==    by 0x44DE72: nmap_main(int, char**) (nmap.cc:1722)
> ==20569==    by 0x446152: main (main.cc:215)
> ==20569== 
>
> Valgrind is (rightfully) complaining that the source and destination
> address are the same to a call to memcpy().  In looking at the code, it
> seems that they will often be the same:
>
> Starting at tcpip.cc:2977
>
>         sin = (struct sockaddr_in *) &(tmpifr.ifr_addr); /* ifr_netmask only on Linux */
>         addr_stob(&(tmpifr.ifr_addr), &mydevs[numifaces].netmask_bits);
>       }
>
> ...comments...
>
>       Strncpy(tmpifr.ifr_name, ifr->ifr_name, sizeof(tmpifr.ifr_name));
>       memcpy(&(tmpifr.ifr_addr), sin, MIN(sizeof(tmpifr.ifr_addr), sizeof(*sin)\));
>
> Does anybody have any ideas about the right way to fix this code?
> Should we just drop the call to memcpy?

The two lines you cited can be removed because they always copy
something that's already there. There's another similar call to memcpy
down below that I think can also be removed.

memcpy(&(tmpifr.ifr_addr), sin, MIN(sizeof(tmpifr.ifr_addr), MIN(sizeof(tmpifr.ifr_addr), sizeof(*sin))));

Using memmove would remove the error but just removing the lines would
help to reduce the duplication in this function. Notice the
MIN(a, MIN(a, b)) calculation in the line above.

You can remove those lines if you test that doing so isn't obviously
broken.

David Fifield

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org