Re: Something like a bug

[Pablo Neira Ayuso <pablo@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

Rick Xu wrote:
> int nfnl_nfa_addattr_l(struct nfattr *nfa, int maxlen, int type,
>                const void *data, int alen)
> {
>  ......
> 
>     if ((NFA_OK(nfa, nfa->nfa_len) + len) > maxlen) {
>         errno = ENOSPC;
>         return -1;
>     }
> 
>     subnfa = (struct nfattr *)(((char *)nfa) + NFA_OK(nfa, nfa->nfa_len));
> 
> .....
> }
> 
> NFA_OK looks so weird here. I think it should be:
> int nfnl_nfa_addattr_l(struct nfattr *nfa, int maxlen, int type,
>                const void *data, int alen)
> {
>  ......
> 
>     if ( NFA_ALIGN(nfa->nfa_len) + NFA_LENGTH(len) > maxlen) {
>         errno = ENOSPC;
>         return -1;
>     }
> 
>     subnfa = (struct nfattr *)(((char *)nfa) + NFA_ALIGN( nfa->nfa_len));
> 
> .....
> }
> 
> Please kindly correct me if I made a mistake.


Indeed, that function is completely broken but it does not have any
known client.

It seems a leftover of the days when we didn't have anything better to
nest attributes, so that it's completely useless these days. I have
fixed in SVN anyway. Patch attached.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1039 bytes --]

Index: configure.in
===================================================================
--- configure.in	(revisión: 7400)
+++ configure.in	(copia de trabajo)
@@ -4,7 +4,7 @@
 
 AC_CANONICAL_SYSTEM
 
-AM_INIT_AUTOMAKE(libnfnetlink, 0.0.33)
+AM_INIT_AUTOMAKE(libnfnetlink, 0.0.34)
 
 AC_PROG_CC
 AC_EXEEXT
Index: src/libnfnetlink.c
===================================================================
--- src/libnfnetlink.c	(revisión: 7400)
+++ src/libnfnetlink.c	(copia de trabajo)
@@ -799,16 +799,16 @@
 	assert(maxlen > 0);
 	assert(type >= 0);
 
-	if ((NFA_OK(nfa, nfa->nfa_len) + len) > maxlen) {
+	if (NFA_ALIGN(nfa->nfa_len) + len > maxlen) {
 		errno = ENOSPC;
 		return -1;
 	}
 
-	subnfa = (struct nfattr *)(((char *)nfa) + NFA_OK(nfa, nfa->nfa_len));
+	subnfa = (struct nfattr *)(((char *)nfa) + NFA_ALIGN(nfa->nfa_len));
 	subnfa->nfa_type = type;
 	subnfa->nfa_len = len;
 	memcpy(NFA_DATA(subnfa), data, alen);
-	nfa->nfa_len = (NLMSG_ALIGN(nfa->nfa_len) + len);
+	nfa->nfa_len = NFA_ALIGN(nfa->nfa_len) + len;
 
 	return 0;
 }