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."

Attachment: signature.asc
Description: Digital signature

Reply via email to