* [libnftnl PATCH v3] utils: fix arp family number
@ 2014-10-20 11:52 Arturo Borrero Gonzalez
2014-10-21 7:59 ` Pablo Neira Ayuso
0 siblings, 1 reply; 6+ messages in thread
From: Arturo Borrero Gonzalez @ 2014-10-20 11:52 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo
NFPROTO_ARP = 3 in kernel space.
We need the same value here in userspace in order to correctly communicate
with the kernel.
The failure solved by this patch made that {XML|JSON}-parsed tables of ARP
family unable to be directly injected into kernel.
To prevent future errors, this patch changes raw and AF_* values by the mathing
NFPROTO_* couterpart as seen in linux/netfilter.h in both functions:
* nft_family2str()
* nft_str2family()
Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
---
v2: rework+fix using the array-matching approach suggested by Pablo.
v3: constify both the pointer and the data, suggested by Jan.
Keep setting errno to EAFNOSUPPORT in nft_str2family().
src/utils.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/src/utils.c b/src/utils.c
index d70fbf1..9013b68 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -20,36 +20,33 @@
#include <linux/netfilter.h>
#include <linux/netfilter/nf_tables.h>
+static const char *const nft_family_str[NFPROTO_NUMPROTO] = {
+ [NFPROTO_INET] = "inet",
+ [NFPROTO_IPV4] = "ip",
+ [NFPROTO_ARP] = "arp",
+ [NFPROTO_BRIDGE] = "bridge",
+ [NFPROTO_IPV6] = "ip6",
+};
+
const char *nft_family2str(uint32_t family)
{
- switch (family) {
- case AF_INET:
- return "ip";
- case AF_INET6:
- return "ip6";
- case 1:
- return "inet";
- case AF_BRIDGE:
- return "bridge";
- case 3: /* NFPROTO_ARP */
- return "arp";
- default:
+ if (nft_family_str[family] == NULL)
return "unknown";
- }
+
+ return nft_family_str[family];
}
int nft_str2family(const char *family)
{
- if (strcmp(family, "ip") == 0)
- return AF_INET;
- else if (strcmp(family, "ip6") == 0)
- return AF_INET6;
- else if (strcmp(family, "inet") == 0)
- return 1;
- else if (strcmp(family, "bridge") == 0)
- return AF_BRIDGE;
- else if (strcmp(family, "arp") == 0)
- return 0;
+ int i;
+
+ for (i = 0; i < NFPROTO_NUMPROTO; i++) {
+ if (nft_family_str[i] == NULL)
+ continue;
+
+ if (strcmp(nft_family_str[i], family) == 0)
+ return i;
+ }
errno = EAFNOSUPPORT;
return -1;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [libnftnl PATCH v3] utils: fix arp family number 2014-10-20 11:52 [libnftnl PATCH v3] utils: fix arp family number Arturo Borrero Gonzalez @ 2014-10-21 7:59 ` Pablo Neira Ayuso 2014-10-21 8:53 ` Arturo Borrero Gonzalez 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2014-10-21 7:59 UTC (permalink / raw) To: Arturo Borrero Gonzalez; +Cc: netfilter-devel On Mon, Oct 20, 2014 at 01:52:18PM +0200, Arturo Borrero Gonzalez wrote: > NFPROTO_ARP = 3 in kernel space. > > We need the same value here in userspace in order to correctly communicate > with the kernel. > > The failure solved by this patch made that {XML|JSON}-parsed tables of ARP > family unable to be directly injected into kernel. > > To prevent future errors, this patch changes raw and AF_* values by the mathing > NFPROTO_* couterpart as seen in linux/netfilter.h in both functions: > * nft_family2str() > * nft_str2family() Applied, thanks. BTW, it would be good to see a similar refactor in nft_verdict2str(). ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH v3] utils: fix arp family number 2014-10-21 7:59 ` Pablo Neira Ayuso @ 2014-10-21 8:53 ` Arturo Borrero Gonzalez 2014-10-21 9:56 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Arturo Borrero Gonzalez @ 2014-10-21 8:53 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list On 21 October 2014 09:59, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > BTW, it would be good to see a similar refactor in nft_verdict2str(). I don't see a clean way to do it, given some verdicts are negative numbers (enum nft_verdicts in nf_tables.h). We may end accessing a negative index, out of bounds of the array. -- Arturo Borrero González -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH v3] utils: fix arp family number 2014-10-21 8:53 ` Arturo Borrero Gonzalez @ 2014-10-21 9:56 ` Pablo Neira Ayuso 2014-10-21 9:57 ` Pablo Neira Ayuso 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2014-10-21 9:56 UTC (permalink / raw) To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list On Tue, Oct 21, 2014 at 10:53:19AM +0200, Arturo Borrero Gonzalez wrote: > On 21 October 2014 09:59, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > BTW, it would be good to see a similar refactor in nft_verdict2str(). > > I don't see a clean way to do it, given some verdicts are negative > numbers (enum nft_verdicts in nf_tables.h). > We may end accessing a negative index, out of bounds of the array. I see, you mean: enum nft_verdicts { NFT_CONTINUE = -1, NFT_BREAK = -2, NFT_JUMP = -3, NFT_GOTO = -4, NFT_RETURN = -5, }; You can add some function to shift the values: #define nft_verdict_index(base) (base + 5) ... nft_verdict_array[] = { [nft_verdict_index(NFT_RETURN)] = "return", ... }; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH v3] utils: fix arp family number 2014-10-21 9:56 ` Pablo Neira Ayuso @ 2014-10-21 9:57 ` Pablo Neira Ayuso 2014-10-22 14:24 ` Arturo Borrero Gonzalez 0 siblings, 1 reply; 6+ messages in thread From: Pablo Neira Ayuso @ 2014-10-21 9:57 UTC (permalink / raw) To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list On Tue, Oct 21, 2014 at 11:56:49AM +0200, Pablo Neira Ayuso wrote: > On Tue, Oct 21, 2014 at 10:53:19AM +0200, Arturo Borrero Gonzalez wrote: > > On 21 October 2014 09:59, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > > > > > BTW, it would be good to see a similar refactor in nft_verdict2str(). > > > > I don't see a clean way to do it, given some verdicts are negative > > numbers (enum nft_verdicts in nf_tables.h). > > We may end accessing a negative index, out of bounds of the array. > > I see, you mean: > > enum nft_verdicts { > NFT_CONTINUE = -1, > NFT_BREAK = -2, > NFT_JUMP = -3, > NFT_GOTO = -4, > NFT_RETURN = -5, > }; > > You can add some function to shift the values: > > #define nft_verdict_index(base) (base + 5) BTW, instead of 5, add: #define NFT_VERDICT_BASE NFT_RETURN and use it. > > ... nft_verdict_array[] = { > [nft_verdict_index(NFT_RETURN)] = "return", > ... > }; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [libnftnl PATCH v3] utils: fix arp family number 2014-10-21 9:57 ` Pablo Neira Ayuso @ 2014-10-22 14:24 ` Arturo Borrero Gonzalez 0 siblings, 0 replies; 6+ messages in thread From: Arturo Borrero Gonzalez @ 2014-10-22 14:24 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list On 21 October 2014 11:57, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Tue, Oct 21, 2014 at 11:56:49AM +0200, Pablo Neira Ayuso wrote: >> On Tue, Oct 21, 2014 at 10:53:19AM +0200, Arturo Borrero Gonzalez wrote: >> > On 21 October 2014 09:59, Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> > > >> > > BTW, it would be good to see a similar refactor in nft_verdict2str(). >> > >> > I don't see a clean way to do it, given some verdicts are negative >> > numbers (enum nft_verdicts in nf_tables.h). >> > We may end accessing a negative index, out of bounds of the array. >> >> I see, you mean: >> >> enum nft_verdicts { >> NFT_CONTINUE = -1, >> NFT_BREAK = -2, >> NFT_JUMP = -3, >> NFT_GOTO = -4, >> NFT_RETURN = -5, >> }; >> >> You can add some function to shift the values: >> >> #define nft_verdict_index(base) (base + 5) > > BTW, instead of 5, add: > > #define NFT_VERDICT_BASE NFT_RETURN > > and use it. > >> >> ... nft_verdict_array[] = { >> [nft_verdict_index(NFT_RETURN)] = "return", >> ... >> }; Ok, following your idea, I end with something like this: /* * NF_DROP = 0 * NF_ACCEPT = 1 * NFT_JUMP = -3 * NFT_GOTO = -4 * NFT_RETURN = -5 */ #define NFT_VERDICT_BASE -NFT_RETURN #define nft_verdict_index(base) (base + NFT_VERDICT_BASE) #define NFT_VERDICT_ARRAY_LEN nft_verdict_index(NF_ACCEPT) static const char *const nft_verdict_str[NFT_VERDICT_ARRAY_LEN + 1] = { [nft_verdict_index(NFT_RETURN)] = "return", /* 0 */ [nft_verdict_index(NFT_GOTO)] = "goto", /* 1 */ [nft_verdict_index(NFT_JUMP)] = "jump", /* 2 */ [nft_verdict_index(NF_DROP)] = "drop", /* 5 */ [nft_verdict_index(NF_ACCEPT)] = "accept", /* 6 */ }; const char *nft_verdict2str(int verdict) { if (nft_verdict_str[nft_verdict_index(verdict)] == NULL) return "unknown"; return nft_verdict_str[nft_verdict_index(verdict)]; } int nft_str2verdict(const char *verdict, int *verdict_num) { int i; for (i = 0; i < NFT_VERDICT_ARRAY_LEN; i++) { if (nft_verdict_str[i] == NULL) continue; if (strcmp(nft_verdict_str[i], verdict) == 0) { *verdict_num = nft_verdict_index(i); return 0; } } return -1; } I think the current code (the switch based one) is better, don't you? That array seems very error prone. -- Arturo Borrero González -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-22 14:24 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-10-20 11:52 [libnftnl PATCH v3] utils: fix arp family number Arturo Borrero Gonzalez 2014-10-21 7:59 ` Pablo Neira Ayuso 2014-10-21 8:53 ` Arturo Borrero Gonzalez 2014-10-21 9:56 ` Pablo Neira Ayuso 2014-10-21 9:57 ` Pablo Neira Ayuso 2014-10-22 14:24 ` Arturo Borrero Gonzalez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).