]> git.puffer.fish Git - matthieu/frr.git/commit
bgpd: memmove needed in community_del_val
authorJohn Glotzer <glotzer@amazon.com>
Mon, 4 Aug 2014 19:39:23 +0000 (19:39 +0000)
committerDavid Lamparter <equinox@opensourcerouting.org>
Sun, 17 Aug 2014 23:52:26 +0000 (01:52 +0200)
commit4c005e3f65a1f5b4592b1ebbac392cbb1a710998
treeec2feadf6fe20841a179d31714a053863919ae1e
parent3ef0b877f08344aa52367794aa4ec32b12becd6d
bgpd: memmove needed in community_del_val

In bgpd/bgp_community_del_val memcpy is used for potentially overlapping
regions which is *not* safe. It may "work" in some cases but is not
guaranteed to work in all cases. The case that I saw fail was on an
x86_64 architecture with the number of bytes being moved/copied equal to
8.

The way the code is written the uint32_t pointers will always differ by
1, which is equivalent to a memcpy/memmove of regions that are 4 bytes
away from one another. So the code failed while copying an 8 byte region
to an address that is 4 bytes lower i.e. overlapping regions.

Interestingly, the same architecture had no problems with a 12 byte
copy.

When the code failed the communities were [200,300,400] and a call was
made to delete the 200 community. The result of this was an array that
looked like [400,400] which was uniquified to [400]. Of course the
expected result should have been [300, 400].

One additional point - in our production environment memmove would not
*link* without including <string.h> but in an isolated quagga git repo
this #include does not seem to be required and I see memmove is used in
vtysh.c without this #include either.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
bgpd/bgp_community.c