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: santi...@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."
signature.asc
Description: Digital signature