1 commit 9eda90ce8094683a5315007fbd0f9249a284f36f |
|
2 Author: Paul Jakma <[email protected]> |
|
3 Date: Thu Aug 30 13:36:17 2007 +0000 |
|
4 |
|
5 [bgpd] bug #398 Bogus free on out route-map, and assert() with rsclients |
|
6 |
|
7 2007-08-27 Paul Jakma <[email protected]> |
|
8 |
|
9 * bgp_route.c: (bgp_announce_check) Fix bug #398, slight |
|
10 modification of Vladimir Ivanov's suggested fix - to keep |
|
11 memory alloc conditional. |
|
12 (bgp_process_announce_selected) Don't take struct attr as |
|
13 argument, none of the callers need it and it needlessly |
|
14 distances allocation from use. |
|
15 Free the extended attr, the attr itself is on the stack. |
|
16 Fix bad indentation. |
|
17 * bgp_attr.c: (bgp_packet_attribute) Remove incorrect assert, |
|
18 and adjust conditional to test attr->extra, diagnosis by |
|
19 Vladimir Ivanov in bug #398. |
|
20 |
|
21 2007-08-27 Vladimir Ivanov <[email protected]> |
|
22 |
|
23 * bgp_route.c: (bgp_announce_check_rsclient) copy of |
|
24 ri->attr is no longer deep enough, due to addition of |
|
25 attr->extra. It should use bgp_attr_dup, as |
|
26 bgp_announce_check() does. |
|
27 |
|
28 diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c |
|
29 index 23d9586..ee17b6d 100644 |
|
30 --- bgpd/bgp_attr.c |
|
31 +++ bgpd/bgp_attr.c |
|
32 @@ -1625,8 +1625,6 @@ |
|
33 && from |
|
34 && peer_sort (from) == BGP_PEER_IBGP) |
|
35 { |
|
36 - assert (attr->extra); |
|
37 - |
|
38 /* Originator ID. */ |
|
39 stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); |
|
40 stream_putc (s, BGP_ATTR_ORIGINATOR_ID); |
|
41 @@ -1641,7 +1639,7 @@ |
|
42 stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); |
|
43 stream_putc (s, BGP_ATTR_CLUSTER_LIST); |
|
44 |
|
45 - if (attr->extra->cluster) |
|
46 + if (attr->extra && attr->extra->cluster) |
|
47 { |
|
48 stream_putc (s, attr->extra->cluster->length + 4); |
|
49 /* If this peer configuration's parent BGP has cluster_id. */ |
|
50 diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c |
|
51 index 0f4da98..9ddeca5 100644 |
|
52 --- bgpd/bgp_route.c |
|
53 +++ bgpd/bgp_route.c |
|
54 @@ -999,11 +999,10 @@ |
|
55 || (ri->extra && ri->extra->suppress) ) |
|
56 { |
|
57 struct bgp_info info; |
|
58 - struct attr dummy_attr; |
|
59 + struct attr dummy_attr = { 0 }; |
|
60 |
|
61 info.peer = peer; |
|
62 info.attr = attr; |
|
63 - |
|
64 |
|
65 /* The route reflector is not allowed to modify the attributes |
|
66 of the reflected IBGP routes. */ |
|
67 @@ -1010,9 +1009,8 @@ |
|
68 if (peer_sort (from) == BGP_PEER_IBGP |
|
69 && peer_sort (peer) == BGP_PEER_IBGP) |
|
70 { |
|
71 - dummy_attr.extra = NULL; |
|
72 bgp_attr_dup (&dummy_attr, attr); |
|
73 - info.attr = &dummy_attr; |
|
74 + info.attr = &dummy_attr; |
|
75 } |
|
76 |
|
77 SET_FLAG (peer->rmap_type, PEER_RMAP_TYPE_OUT); |
|
78 @@ -1024,7 +1022,8 @@ |
|
79 |
|
80 peer->rmap_type = 0; |
|
81 |
|
82 - bgp_attr_extra_free (&dummy_attr); |
|
83 + if (dummy_attr.extra) |
|
84 + bgp_attr_extra_free (&dummy_attr); |
|
85 |
|
86 if (ret == RMAP_DENYMATCH) |
|
87 { |
|
88 @@ -1127,7 +1126,7 @@ |
|
89 #endif /* BGP_SEND_ASPATH_CHECK */ |
|
90 |
|
91 /* For modify attribute, copy it to temporary structure. */ |
|
92 - *attr = *ri->attr; |
|
93 + bgp_attr_dup (attr, ri->attr); |
|
94 |
|
95 /* next-hop-set */ |
|
96 if ((p->family == AF_INET && attr->nexthop.s_addr == 0) |
|
97 @@ -1329,21 +1328,22 @@ |
|
98 |
|
99 static int |
|
100 bgp_process_announce_selected (struct peer *peer, struct bgp_info *selected, |
|
101 - struct bgp_node *rn, struct attr *attr, afi_t afi, safi_t safi) |
|
102 - { |
|
103 + struct bgp_node *rn, afi_t afi, safi_t safi) |
|
104 +{ |
|
105 struct prefix *p; |
|
106 + struct attr attr = { 0 }; |
|
107 |
|
108 p = &rn->p; |
|
109 |
|
110 - /* Announce route to Established peer. */ |
|
111 - if (peer->status != Established) |
|
112 + /* Announce route to Established peer. */ |
|
113 + if (peer->status != Established) |
|
114 return 0; |
|
115 |
|
116 - /* Address family configuration check. */ |
|
117 - if (! peer->afc_nego[afi][safi]) |
|
118 + /* Address family configuration check. */ |
|
119 + if (! peer->afc_nego[afi][safi]) |
|
120 return 0; |
|
121 |
|
122 - /* First update is deferred until ORF or ROUTE-REFRESH is received */ |
|
123 + /* First update is deferred until ORF or ROUTE-REFRESH is received */ |
|
124 if (CHECK_FLAG (peer->af_sflags[afi][safi], |
|
125 PEER_STATUS_ORF_WAIT_REFRESH)) |
|
126 return 0; |
|
127 @@ -1353,8 +1353,8 @@ |
|
128 case BGP_TABLE_MAIN: |
|
129 /* Announcement to peer->conf. If the route is filtered, |
|
130 withdraw it. */ |
|
131 - if (selected && bgp_announce_check (selected, peer, p, attr, afi, safi)) |
|
132 - bgp_adj_out_set (rn, peer, p, attr, afi, safi, selected); |
|
133 + if (selected && bgp_announce_check (selected, peer, p, &attr, afi, safi)) |
|
134 + bgp_adj_out_set (rn, peer, p, &attr, afi, safi, selected); |
|
135 else |
|
136 bgp_adj_out_unset (rn, peer, p, afi, safi); |
|
137 break; |
|
138 @@ -1361,13 +1361,16 @@ |
|
139 case BGP_TABLE_RSCLIENT: |
|
140 /* Announcement to peer->conf. If the route is filtered, |
|
141 withdraw it. */ |
|
142 - if (selected && bgp_announce_check_rsclient |
|
143 - (selected, peer, p, attr, afi, safi)) |
|
144 - bgp_adj_out_set (rn, peer, p, attr, afi, safi, selected); |
|
145 - else |
|
146 - bgp_adj_out_unset (rn, peer, p, afi, safi); |
|
147 + if (selected && |
|
148 + bgp_announce_check_rsclient (selected, peer, p, &attr, afi, safi)) |
|
149 + bgp_adj_out_set (rn, peer, p, &attr, afi, safi, selected); |
|
150 + else |
|
151 + bgp_adj_out_unset (rn, peer, p, afi, safi); |
|
152 break; |
|
153 } |
|
154 + |
|
155 + bgp_attr_extra_free (&attr); |
|
156 + |
|
157 return 0; |
|
158 } |
|
159 |
|
160 @@ -1417,8 +1420,7 @@ |
|
161 bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED); |
|
162 } |
|
163 |
|
164 - bgp_process_announce_selected (rsclient, new_select, rn, &attr, |
|
165 - afi, safi); |
|
166 + bgp_process_announce_selected (rsclient, new_select, rn, afi, safi); |
|
167 } |
|
168 } |
|
169 else |
|
170 @@ -1430,8 +1432,7 @@ |
|
171 bgp_info_set_flag (rn, new_select, BGP_INFO_SELECTED); |
|
172 bgp_info_unset_flag (rn, new_select, BGP_INFO_ATTR_CHANGED); |
|
173 } |
|
174 - bgp_process_announce_selected (rsclient, new_select, rn, |
|
175 - &attr, afi, safi); |
|
176 + bgp_process_announce_selected (rsclient, new_select, rn, afi, safi); |
|
177 } |
|
178 |
|
179 if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED)) |
|
180 @@ -1457,10 +1458,7 @@ |
|
181 struct bgp_info_pair old_and_new; |
|
182 struct listnode *node, *nnode; |
|
183 struct peer *peer; |
|
184 - struct attr attr; |
|
185 |
|
186 - memset (&attr, 0, sizeof (struct attr)); |
|
187 - |
|
188 /* Best path selection. */ |
|
189 bgp_best_selection (bgp, rn, &old_and_new); |
|
190 old_select = old_and_new.old; |
|
191 @@ -1491,7 +1489,7 @@ |
|
192 /* Check each BGP peer. */ |
|
193 for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) |
|
194 { |
|
195 - bgp_process_announce_selected (peer, new_select, rn, &attr, afi, safi); |
|
196 + bgp_process_announce_selected (peer, new_select, rn, afi, safi); |
|
197 } |
|
198 |
|
199 /* FIB update. */ |
|
200 @@ -1516,8 +1514,6 @@ |
|
201 if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED)) |
|
202 bgp_info_reap (rn, old_select); |
|
203 |
|
204 - bgp_attr_extra_free (&attr); |
|
205 - |
|
206 UNSET_FLAG (rn->flags, BGP_NODE_PROCESS_SCHEDULED); |
|
207 return WQ_SUCCESS; |
|
208 } |
|
209 @@ -5888,7 +5884,7 @@ |
|
210 { |
|
211 struct route_map *rmap = output_arg; |
|
212 struct bgp_info binfo; |
|
213 - struct attr dummy_attr; |
|
214 + struct attr dummy_attr = { 0 }; |
|
215 int ret; |
|
216 |
|
217 bgp_attr_dup (&dummy_attr, ri->attr); |
|