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 <[email protected]>
---
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;
}
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.
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.