| Message ID | 20160812201722.10134-1-carlosfg@riseup.net |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Fri, Aug 12, 2016 at 10:17:19PM +0200, Carlos Falgueras García wrote: > Changes random values for macros because the conversion to string of these > values are performed by accessing to an array of strings. Then, we should fix the functions to return "unknown" for out of bound access of the array. I think you have to fix this in the library, not the test. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/13/2016 12:12 PM, Pablo Neira Ayuso wrote: > On Fri, Aug 12, 2016 at 10:17:19PM +0200, Carlos Falgueras García wrote: >> Changes random values for macros because the conversion to string of these >> values are performed by accessing to an array of strings. > > Then, we should fix the functions to return "unknown" for out of bound > access of the array. > > I think you have to fix this in the library, not the test. The most common structure we have is this: static const char *element2str_array[MAX_ELEMENTS] { [ELEMENT_1] = "element 1", ... [ELEMENT_N] = "element N", }; static const char *element2str(uint32_t element) { if (element < MAX_ELEMENT) return "unkown"; return element2str_array[element]; } I think it is better if we change it to a switch-case statement. I think a switch-case have the same performance and it is more robust due cases like this: netfilter.h: enum { NFPROTO_UNSPEC = 0, NFPROTO_INET = 1, NFPROTO_IPV4 = 2, NFPROTO_ARP = 3, NFPROTO_NETDEV = 5, NFPROTO_BRIDGE = 7, NFPROTO_IPV6 = 10, NFPROTO_DECNET = 12, NFPROTO_NUMPROTO, }; utils.c: static const char *const nftnl_family_str[NFPROTO_NUMPROTO] = { [NFPROTO_INET] = "inet", [NFPROTO_IPV4] = "ip", [NFPROTO_ARP] = "arp", [NFPROTO_NETDEV] = "netdev", [NFPROTO_BRIDGE] = "bridge", [NFPROTO_IPV6] = "ip6", }; We do not have all element in the translation table and elements have unconsecutives values. Another possible solution is something like this: static const char *element2str(uint32_t element) { - if (element < MAX_ELEMENT) + if (element < MAX_ELEMENT || !element2str_array[element]) return "unkown"; return element2str_array[element]; } What do you think? Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Aug 13, 2016 at 05:25:19PM +0200, Carlos Falgueras García wrote: > Another possible solution is something like this: > > static const char *element2str(uint32_t element) { > - if (element < MAX_ELEMENT) > + if (element < MAX_ELEMENT || !element2str_array[element]) > return "unkown"; > return element2str_array[element]; > } > > What do you think? This only requires a oneliner patch and a very simple solution, please do this. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/nft-expr_cmp-test.c b/tests/nft-expr_cmp-test.c index ec00bb9..4271940 100644 --- a/tests/nft-expr_cmp-test.c +++ b/tests/nft-expr_cmp-test.c @@ -64,7 +64,7 @@ int main(int argc, char *argv[]) nftnl_expr_set(ex, NFTNL_EXPR_CMP_DATA, &data_len, sizeof(data_len)); nftnl_expr_set_u32(ex, NFTNL_EXPR_CMP_SREG, 0x12345678); - nftnl_expr_set_u32(ex, NFTNL_EXPR_CMP_OP, 0x78123456); + nftnl_expr_set_u32(ex, NFTNL_EXPR_CMP_OP, NFT_CMP_LT); nftnl_rule_add_expr(a, ex); diff --git a/tests/nft-expr_nat-test.c b/tests/nft-expr_nat-test.c index fd3a488..9c15a0a 100644 --- a/tests/nft-expr_nat-test.c +++ b/tests/nft-expr_nat-test.c @@ -71,8 +71,8 @@ int main(int argc, char *argv[]) if (ex == NULL) print_err("OOM"); - nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_TYPE, 0x1234568); - nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_FAMILY, 0x3456721); + nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_TYPE, NFT_NAT_SNAT); + nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_FAMILY, NFPROTO_INET); nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_REG_ADDR_MIN, 0x1452638); nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_REG_ADDR_MAX, 0x5134682); nftnl_expr_set_u32(ex, NFTNL_EXPR_NAT_REG_PROTO_MIN, 0x6124385);
Changes random values for macros because the conversion to string of these values are performed by accessing to an array of strings. Signed-off-by: Carlos Falgueras García <carlosfg@riseup.net> --- tests/nft-expr_cmp-test.c | 2 +- tests/nft-expr_nat-test.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)