From fdb89ab51a83cd4cb1c4ce3068eba5e3a431954b Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:01:49 +0200 Subject: [PATCH 1/3] zebra: Remove dead SRv6 code At line 1736, `alloc_mode` is set to `SRV6_SID_ALLOC_MODE_EXPLICIT` or `SRV6_SID_ALLOC_MODE_DYNAMIC` depending on the `sid_value` variable. There will never be a case where alloc_mode will be `SRV6_SID_ALLOC_MODE_MAX` or `SRV6_SID_ALLOC_MODE_UNSPEC`. Let's replace the `switch(alloc_mode) {...}` with an if-else. Fixes CID 1594015. ** CID 1594015: (DEADCODE) /zebra/zebra_srv6.c: 1782 in get_srv6_sid() /zebra/zebra_srv6.c: 1781 in get_srv6_sid() ________________________________________________________________________________________________________ *** CID 1594015: (DEADCODE) /zebra/zebra_srv6.c: 1782 in get_srv6_sid() 1776 } 1777 1778 ret = get_srv6_sid_dynamic(sid, ctx, locator); 1779 1780 break; 1781 case SRV6_SID_ALLOC_MODE_MAX: CID 1594015: (DEADCODE) Execution cannot reach this statement: "case SRV6_SID_ALLOC_MODE_UN...". 1782 case SRV6_SID_ALLOC_MODE_UNSPEC: 1783 default: 1784 flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, 1785 "%s: SRv6 Manager: Unrecognized alloc mode %u", 1786 __func__, alloc_mode); 1787 /* We should never arrive here */ /zebra/zebra_srv6.c: 1781 in get_srv6_sid() 1775 return -1; 1776 } 1777 1778 ret = get_srv6_sid_dynamic(sid, ctx, locator); 1779 1780 break; CID 1594015: (DEADCODE) Execution cannot reach this statement: "case SRV6_SID_ALLOC_MODE_MAX:". 1781 case SRV6_SID_ALLOC_MODE_MAX: 1782 case SRV6_SID_ALLOC_MODE_UNSPEC: 1783 default: 1784 flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, 1785 "%s: SRv6 Manager: Unrecognized alloc mode %u", 1786 __func__, alloc_mode); Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index 0ca77a4974..2c1f86a3fc 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -1742,8 +1742,7 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, __func__, srv6_sid_ctx2str(buf, sizeof(buf), ctx), sid_value, srv6_sid_alloc_mode2str(alloc_mode)); - switch (alloc_mode) { - case SRV6_SID_ALLOC_MODE_EXPLICIT: + if (alloc_mode == SRV6_SID_ALLOC_MODE_EXPLICIT) { /* * Explicit SID allocation: allocate a specific SID value */ @@ -1755,9 +1754,7 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, } ret = get_srv6_sid_explicit(sid, ctx, sid_value); - - break; - case SRV6_SID_ALLOC_MODE_DYNAMIC: + } else { /* * Dynamic SID allocation: allocate any available SID value */ @@ -1776,16 +1773,6 @@ int get_srv6_sid(struct zebra_srv6_sid **sid, struct srv6_sid_ctx *ctx, } ret = get_srv6_sid_dynamic(sid, ctx, locator); - - break; - case SRV6_SID_ALLOC_MODE_MAX: - case SRV6_SID_ALLOC_MODE_UNSPEC: - default: - flog_err(EC_ZEBRA_SM_CANNOT_ASSIGN_SID, - "%s: SRv6 Manager: Unrecognized alloc mode %u", - __func__, alloc_mode); - /* We should never arrive here */ - assert(0); } return ret; From 375a02d2a30cd7a06b568187e23226ad5d083c87 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:41:34 +0200 Subject: [PATCH 2/3] zebra: Fix wrong variable used in `for` loop The `for` loop starting at line 1848 searches the `func_allocated` array for a pointer that points to a specific `sid_wide_func` element. The loop should iterate over all the elements of the `func_allocated` array and dereference each element to see if it is the one we are looking for. Currently, the loop is using the wrong variable to iterate over the array. Let's fix this issue by using the correct variable in the loop. Fixes CID 1594014 Fixes CID 1594016 ** CID 1594014: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_srv6.c: 1860 in release_srv6_sid_func_explicit() ________________________________________________________________________________________________________ *** CID 1594014: Null pointer dereferences (FORWARD_NULL) /zebra/zebra_srv6.c: 1860 in release_srv6_sid_func_explicit() 1854 1855 /* Lookup SID function in the functions allocated list of EWLIB range */ 1856 for (ALL_LIST_ELEMENTS_RO(block->u.usid 1857 .wide_lib[sid_func] 1858 .func_allocated, 1859 node, sid_func_ptr)) CID 1594014: Null pointer dereferences (FORWARD_NULL) Dereferencing null pointer "sid_wide_func_ptr". 1860 if (*sid_wide_func_ptr == sid_wide_func) 1861 break; 1862 1863 /* Ensure that the SID function is allocated */ 1864 if (!sid_wide_func_ptr) { 1865 zlog_warn("%s: failed to release wide SID function %u, function is not allocated", ** CID 1594016: Possible Control flow issues (DEADCODE) /zebra/zebra_srv6.c: 1871 in release_srv6_sid_func_explicit() ________________________________________________________________________________________________________ *** CID 1594016: Possible Control flow issues (DEADCODE) /zebra/zebra_srv6.c: 1871 in release_srv6_sid_func_explicit() 1865 zlog_warn("%s: failed to release wide SID function %u, function is not allocated", 1866 __func__, sid_wide_func); 1867 return -1; 1868 } 1869 1870 /* Release the SID function from the EWLIB range */ CID 1594016: Possible Control flow issues (DEADCODE) Execution cannot reach this statement: "listnode_delete(block->u.us...". 1871 listnode_delete(block->u.usid.wide_lib[sid_func] 1872 .func_allocated, 1873 sid_wide_func_ptr); 1874 zebra_srv6_sid_func_free(sid_wide_func_ptr); 1875 } else { 1876 zlog_warn("%s: function %u is outside ELIB [%u/%u] and EWLIB alloc ranges [%u/%u]", Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index 2c1f86a3fc..be335a5ded 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -1843,7 +1843,7 @@ static bool release_srv6_sid_func_explicit(struct zebra_srv6_sid_block *block, for (ALL_LIST_ELEMENTS_RO(block->u.usid .wide_lib[sid_func] .func_allocated, - node, sid_func_ptr)) + node, sid_wide_func_ptr)) if (*sid_wide_func_ptr == sid_wide_func) break; From df97a9d13318f15c59bb055b90529e9e8378a619 Mon Sep 17 00:00:00 2001 From: Carmine Scarpitta Date: Fri, 21 Jun 2024 17:47:46 +0200 Subject: [PATCH 3/3] zebra: Fix NULL pointer dereference The `locator` pointer is dereferenced before ensuring it is not NULL. Fix the issue by checking that the pointer is not NULL before dereferencing it. Fixes 1594013 ** CID 1594013: Null pointer dereferences (REVERSE_INULL) /zebra/zebra_srv6.c: 961 in zebra_srv6_sid_compose() ________________________________________________________________________________________________________ *** CID 1594013: Null pointer dereferences (REVERSE_INULL) /zebra/zebra_srv6.c: 961 in zebra_srv6_sid_compose() 955 struct srv6_locator *locator, 956 uint32_t sid_func) 957 { 958 uint8_t offset, func_len; 959 struct srv6_sid_format *format = locator->sid_format; 960 CID 1594013: Null pointer dereferences (REVERSE_INULL) Null-checking "locator" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 961 if (!sid_value || !locator) 962 return false; 963 964 if (format) { 965 offset = format->block_len + format->node_len; 966 func_len = format->function_len; Signed-off-by: Carmine Scarpitta --- zebra/zebra_srv6.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zebra/zebra_srv6.c b/zebra/zebra_srv6.c index be335a5ded..e82b781c6f 100644 --- a/zebra/zebra_srv6.c +++ b/zebra/zebra_srv6.c @@ -956,11 +956,12 @@ static bool zebra_srv6_sid_compose(struct in6_addr *sid_value, uint32_t sid_func) { uint8_t offset, func_len; - struct srv6_sid_format *format = locator->sid_format; + struct srv6_sid_format *format; if (!sid_value || !locator) return false; + format = locator->sid_format; if (format) { offset = format->block_len + format->node_len; func_len = format->function_len;