On Thu, Sep 01, 2011 at 06:05:38PM +0400, Alexander V. Chernikov wrote:
Hello list!
I've got several questions after some attribute code digging:
1) We're calling custom formatting function bgp_get_attr() with wrong lentgh (attribute name is not taken into account). This probably never will cause buffer overflow, but still: if we're passing buffer length it should be real length. Possible simple fix is in bgp_get_attr.diff
OK, but there is probably a bug in the patch: + buf += len; + if (d->format && (len > 2)) There should be probably (buflen - len) > 2 Or perhaps: buf += len; buflen -= len; I will merge and fix that.
2) We know that either no extended attributes (route withdraw) or a pack (ORIGIN, AS_PATH, NEXTHOP) of mandatory attributes (and possibly some other) are passed within BGP UPDATE message. However we're allocating ea_list with single extended attribute on every BGP attribute we parse in bgp_decode_attr(). Maybe we can pre-allocate more than one attribute in ea_list ? Example implementation can be found in attached bgp_attr_ea.diff
This is IMHO unnecessary complication. lp_alloc() is usually very fast and the extended attributes are later merged and sorted in rta_lookup(). -- Elen sila lumenn' omentielvo Ondrej 'SanTiago' Zajicek (email: santiago@crfreenet.org) OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net) "To err is human -- to blame it on a computer is even more so."