On 02/09/10 11:21, rui.sousa@xxxxxxxxxxxxx wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote on 09/02/2010 10:57:39 AM:
>
>> Hi Rui,
>
> Hi Pablo,
>
>> On 01/09/10 15:45, rui.sousa@xxxxxxxxxxxxx wrote:
>>> Hi,
>>>
>>> I have an application using libnetfilter_conntrack-0.100 that started
>>> reporting errors after the commit:
>>>
>>> 1c450e1595afdc8d1bfabb4f640c9251808426eb.
>>
>> Looking at the source code, this seems to be already fixed in
>> libnetfilter_conntrack 0.0.102, please upgrade to latest.
>
> Hmm... looking at the git tree I see that the __build_conntrack() code is
> still calling __build_protoinfo() unconditionally and inside the function
> we always do:
>
> nest = nfnl_nest(&req->nlh, size, CTA_PROTOINFO);
> nest_proto = nfnl_nest(&req->nlh, size, CTA_PROTOINFO_TCP);
> ...
> nfnl_nest_end(&req->nlh, nest_proto);
> nfnl_nest_end(&req->nlh, nest);
>
> even if none of the ATTR_TCP_xxx bits are set. This is what causes the
> kernel to return -EINVAL
> and ignore the conntrack update. Or am I missing something?
I see, I guess that you're using a Linux kernel <= 2.6.25 since I
couldn't reproduce it with recent kernels. Please, could you give a try
to the following patch?
ct: fix EINVAL if not TCP attributes are set for Linux kernel <= 2.6.25
This patch fixes an EINVAL error that we hit in Linux kernel <= 2.6.25.
Basically, if we send an empty CTA_PROTOINFO_TCP attribute nest, the
kernel returns EINVAL. To fix this, we previously check if there is
any TCP attribute set.
Reported-by: Rui Sousa <rui.sousa@xxxxxxxxxxxxx>
Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
---
src/conntrack/build.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/src/conntrack/build.c b/src/conntrack/build.c
index b878ddd..ec7623d 100644
--- a/src/conntrack/build.c
+++ b/src/conntrack/build.c
@@ -106,6 +106,18 @@ static void __build_protoinfo(struct nfnlhdr *req, size_t size,
switch(ct->tuple[__DIR_ORIG].protonum) {
case IPPROTO_TCP:
+ /* Preliminary attribute check to avoid sending an empty
+ * CTA_PROTOINFO_TCP nest, which results in EINVAL in
+ * Linux kernel <= 2.6.25. */
+ if (!(test_bit(ATTR_TCP_STATE, ct->set) ||
+ test_bit(ATTR_TCP_FLAGS_ORIG, ct->set) ||
+ test_bit(ATTR_TCP_FLAGS_REPL, ct->set) ||
+ test_bit(ATTR_TCP_MASK_ORIG, ct->set) ||
+ test_bit(ATTR_TCP_MASK_REPL, ct->set) ||
+ test_bit(ATTR_TCP_WSCALE_ORIG, ct->set) ||
+ test_bit(ATTR_TCP_WSCALE_REPL, ct->set))) {
+ break;
+ }
nest = nfnl_nest(&req->nlh, size, CTA_PROTOINFO);
nest_proto = nfnl_nest(&req->nlh, size, CTA_PROTOINFO_TCP);
if (test_bit(ATTR_TCP_STATE, ct->set))
@@ -139,6 +151,12 @@ static void __build_protoinfo(struct nfnlhdr *req, size_t size,
nfnl_nest_end(&req->nlh, nest);
break;
case IPPROTO_SCTP:
+ /* See comment above on TCP. */
+ if (!(test_bit(ATTR_SCTP_STATE, ct->set) ||
+ test_bit(ATTR_SCTP_VTAG_ORIG, ct->set) ||
+ test_bit(ATTR_SCTP_VTAG_REPL, ct->set))) {
+ break;
+ }
nest = nfnl_nest(&req->nlh, size, CTA_PROTOINFO);
nest_proto = nfnl_nest(&req->nlh, size, CTA_PROTOINFO_SCTP);
if (test_bit(ATTR_SCTP_STATE, ct->set))
@@ -158,6 +176,12 @@ static void __build_protoinfo(struct nfnlhdr *req, size_t size,
nfnl_nest_end(&req->nlh, nest);
break;
case IPPROTO_DCCP:
+ /* See comment above on TCP. */
+ if (!(test_bit(ATTR_DCCP_STATE, ct->set) ||
+ test_bit(ATTR_DCCP_ROLE, ct->set) ||
+ test_bit(ATTR_DCCP_HANDSHAKE_SEQ, ct->set))) {
+ break;
+ }
nest = nfnl_nest(&req->nlh, size, CTA_PROTOINFO);
nest_proto = nfnl_nest(&req->nlh, size, CTA_PROTOINFO_DCCP);
if (test_bit(ATTR_DCCP_STATE, ct->set))
