Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cid 1636504 #18062

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Cid 1636504 #18062

wants to merge 2 commits into from

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

🟠 Waiting for conditions to match

  • merged [📌 backport requirement]

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

2 participants