Skip to content

Conversation

pguibert6WIND
Copy link
Member

@pguibert6WIND pguibert6WIND commented Feb 7, 2025

The following static analysis can be seen :

> *** CID 1636504:    (ARRAY_VS_SINGLETON)
> /bgpd/bgp_evpn_mh.c: 1241 in bgp_evpn_type1_route_process()
> 1235            build_evpn_type1_prefix(&p, eth_tag, &esi, vtep_ip);
> 1236            /* Process the route. */
> 1237            if (attr) {
> 1238                    bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, safi, ZEBRA_ROUTE_BGP,
> 1239                               BGP_ROUTE_NORMAL, &prd, &label, num_labels, 0, NULL);
> 1240            } else {
> >>>     CID 1636504:    (ARRAY_VS_SINGLETON)
> >>>     Passing "&label" to function "bgp_withdraw" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
> 1241                    bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, ZEBRA_ROUTE_BGP,
> 1242                                 BGP_ROUTE_NORMAL, &prd, &label, num_labels);
> 1243            }
> 1244            return 0;
> 1245     }
> 1246     
> /bgpd/bgp_evpn_mh.c: 1238 in bgp_evpn_type1_route_process()
> 1232             * table
> 1233             */
> 1234            vtep_ip.s_addr = INADDR_ANY;
> 1235            build_evpn_type1_prefix(&p, eth_tag, &esi, vtep_ip);
> 1236            /* Process the route. */
> 1237            if (attr) {
> >>>     CID 1636504:    (ARRAY_VS_SINGLETON)
> >>>     Passing "&label" to function "bgp_update" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
> 1238                    bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, safi, ZEBRA_ROUTE_BGP,
> 1239                               BGP_ROUTE_NORMAL, &prd, &label, num_labels, 0, NULL);
> 1240            } else {
> 1241                    bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, ZEBRA_ROUTE_BGP,
> 1242                                 BGP_ROUTE_NORMAL, &prd, &label, num_labels);
> 1243            }

Fix this by declaring a label array instead of a single array.

@ton31337
Copy link
Member

ton31337 commented Feb 7, 2025

@Mergifyio backport dev/10.3 stable/10.2

Copy link

mergify bot commented Feb 7, 2025

backport dev/10.3 stable/10.2

✅ Backports have been created

@pguibert6WIND pguibert6WIND force-pushed the cid_1636504 branch 2 times, most recently from b180420 to 805691e Compare February 10, 2025 09:00
Remove the num_labels variable, the received bgp_update() and
bgp_withdraw() function will read the message as including one
label or vni value.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
The following static analysis can be seen :

> *** CID 1636504:    (ARRAY_VS_SINGLETON)
> /bgpd/bgp_evpn_mh.c: 1241 in bgp_evpn_type1_route_process()
> 1235            build_evpn_type1_prefix(&p, eth_tag, &esi, vtep_ip);
> 1236            /* Process the route. */
> 1237            if (attr) {
> 1238                    bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, safi, ZEBRA_ROUTE_BGP,
> 1239                               BGP_ROUTE_NORMAL, &prd, &label, num_labels, 0, NULL);
> 1240            } else {
> >>>     CID 1636504:    (ARRAY_VS_SINGLETON)
> >>>     Passing "&label" to function "bgp_withdraw" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
> 1241                    bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, ZEBRA_ROUTE_BGP,
> 1242                                 BGP_ROUTE_NORMAL, &prd, &label, num_labels);
> 1243            }
> 1244            return 0;
> 1245     }
> 1246
> /bgpd/bgp_evpn_mh.c: 1238 in bgp_evpn_type1_route_process()
> 1232             * table
> 1233             */
> 1234            vtep_ip.s_addr = INADDR_ANY;
> 1235            build_evpn_type1_prefix(&p, eth_tag, &esi, vtep_ip);
> 1236            /* Process the route. */
> 1237            if (attr) {
> >>>     CID 1636504:    (ARRAY_VS_SINGLETON)
> >>>     Passing "&label" to function "bgp_update" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
> 1238                    bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, safi, ZEBRA_ROUTE_BGP,
> 1239                               BGP_ROUTE_NORMAL, &prd, &label, num_labels, 0, NULL);
> 1240            } else {
> 1241                    bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, ZEBRA_ROUTE_BGP,
> 1242                                 BGP_ROUTE_NORMAL, &prd, &label, num_labels);
> 1243            }

Fix this by declaring a label array instead of a single array.

Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
@donaldsharp donaldsharp merged commit cea55c9 into FRRouting:master Feb 12, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants