explicitly setting rte->next = null

[Nic Cave-Lynch]

Hi Ondrej

I'm trying to add an attribute on a bgp-advertised route in response to  
some event on my own protocol. When I search for the rte of the route, so  
that I can add the attribute, sometimes ->next is set, so I then look at  
that one to to add the attribute there too. If I don't set ->next = NULL  
in bgp, then there are times when it is invalid, purely as a result of  
garbage in memory, so I end up writing to random places and getting a  
segfault. Or am I approaching the whole thing in the wrong way?

Thanks

Nic C-L

The update function is:


void cst_update_route_from_msg( struct proto * p, ip_addr * prefix, int  
prefix_len, ip_addr * router_addr, bool had_added  )
{
   eattr *ea;
   net * net;
   struct rte *rte;
   struct rte *new_rte;
   rta *temp_rta;
   rta *rta;

   net = net_find( p->table, *prefix, prefix_len);
   if (net)
     {
       rte  = net->routes;
       temp_rta = lp_alloc(cst_linpool, sizeof(struct rta));
       while (rte && rte->attrs)
         {
           memcpy(temp_rta, rte->attrs, sizeof(struct rta));
           temp_rta->aflags  &= ~RTAF_CACHED ;  // say this is uncached
           rta = rta_lookup( temp_rta );

           if (!rta)
               continue;
           new_rte = rte_get_temp(rta_clone(rta));
           if (!new_rte)
               continue;
           new_rte->next = NULL;
           ea = ea_find(new_rte->attrs->eattrs, EA_CODE(EAP_BGP,  
BA_COMMUNITY));
           if (ea)
             {
               u32 asn = cst_get_as_from_rte( rte );
               int found_pub = cst_check_comm_attribs( rte->attrs->eattrs,  
(asn<<16)|PUB_VAL );
               if (!found_pub)
                 {
                   ea->u.ptr = int_set_add(cst_linpool, d,  
(asn<<16)|PUB_VAL);
                   // update the route table with the new attributes...
                   new_rte->net = net;
                   new_rte->pflags |= REF_FILTERED; // make this route not  
be ingored in rte_recalculate
                   rte_update2(rte->attrs->src->proto->main_ahook, net,  
new_rte, rte->attrs->src);
                 }
             }
           rte = rte->next;
         }
     }
   lp_flush(cst_linpool);
}




On Wed, 20 Aug 2014 19:37:38 +1200, Ondrej Zajicek  
<santiago at crfreenet.org> wrote:

> On Wed, Aug 20, 2014 at 05:44:28PM +1200, Nic Cave-Lynch wrote:
>> Hi People
>>
>> I've been trying to find a segfault in a custom protocol I've been  
>> writing.
>> I've finally tracked it down to a situation where bgp protocol, in
>> bgp_rte_update, is calling rte_get_temp but not setting -> next to null,
>> leaving it at whatever was left in RAM when the memory was allocated.  
>> Later,
>> I see it's not null and try to access the ->next and eventually trigger  
>> a
>> segfault.
>
> Generally, rte->next should be set by rte_recalculate() when it is
> inserted to the routing table and should not be accessed before that.
> If this causes crash to you, it is probably some other problem in
> the update path. What are the circumstances when your code accessed
> rte->next value? It was called regularly from rte_announce()?
>