[PATCH] netfilter: NF_HOOK_COND has wrong conditional

[PATCH] netfilter: NF_HOOK_COND has wrong conditional

[Eric Paris]

The NF_HOOK_COND returns 0 when it shouldn't due to what I believe to be an
error in the code as the order of operations is not what was intended.  C will
evalutate == before =.  Which means ret is getting set to the bool result,
rather than the return value of the function call.  The code says

if (ret = function() == 1)
when it meant to say:
if ((ret = function()) == 1)

Normally the compiler would warn, but it doesn't notice it because its
a actually complex conditional and so the wrong code is wrapped in an explict
set of () [exactly what the compiler wants you to do if this was intentional].
Fixing this means that errors when netfilter denies a packet get propagated
back up the stack rather than lost.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 include/linux/netfilter.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 89341c3..03317c8 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -215,7 +215,7 @@ NF_HOOK_COND(uint8_t pf, unsigned int hook, struct sk_buff *skb,
 	int ret;
 
 	if (!cond ||
-	    (ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN) == 1))
+	    ((ret = nf_hook_thresh(pf, hook, skb, in, out, okfn, INT_MIN)) == 1))
 		ret = okfn(skb);
 	return ret;
 }

Re: [PATCH] netfilter: NF_HOOK_COND has wrong conditional

[Jan Engelhardt]

On Thursday 2010-11-11 20:09, Eric Paris wrote:

>The NF_HOOK_COND returns 0 when it shouldn't due to what I believe to be an
>error in the code as the order of operations is not what was intended.  C will
>evalutate == before =.  Which means ret is getting set to the bool result,
>rather than the return value of the function call.  The code says
>
>if (ret = function() == 1)
>when it meant to say:
>if ((ret = function()) == 1)

Thanks for catching. Indeed (ret = f) == 1 is desired, as can be seen in 
patch 2249065f4b22b493bae2caf549b86f175f33188e.

Re: [PATCH] netfilter: NF_HOOK_COND has wrong conditional

[Patrick McHardy]

On 11.11.2010 21:49, Jan Engelhardt wrote:
> On Thursday 2010-11-11 20:09, Eric Paris wrote:
> 
>> The NF_HOOK_COND returns 0 when it shouldn't due to what I believe to be an
>> error in the code as the order of operations is not what was intended.  C will
>> evalutate == before =.  Which means ret is getting set to the bool result,
>> rather than the return value of the function call.  The code says
>>
>> if (ret = function() == 1)
>> when it meant to say:
>> if ((ret = function()) == 1)
> 
> Thanks for catching. Indeed (ret = f) == 1 is desired, as can be seen in 
> patch 2249065f4b22b493bae2caf549b86f175f33188e.

Applied, thanks Eric.