* [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).