Skip to contentSkip to conversation

src: netlink_delinearize: Fix datatype for len

PATCH In review Shivani Bhardwaj 2 participants 3 messages Feb 28, 2016 – Feb 29, 2016 netfilter

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 src: netlink_delinearize: Fix datatype for len

Change the data type of len from unsigned int to int in order to make it valid for checks like

if (len < 0)

The issue was brought into attention by the unexplained behavior of frag with frag-off. Bugzilla entry:
https://bugzilla.netfilter.org/show_bug.cgi?id=935

This patch fixes this bug, however there are still issues with frag that need to be fixed.

---
 src/netlink_delinearize.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
src/netlink_delinearize.c +2 -2
index ae6abb0..2d7a417 100644
@@ -107,7 +107,7 @@ static void netlink_release_registers(struct netlink_parse_ctx *ctx)
 static struct expr *netlink_parse_concat_expr(struct netlink_parse_ctx *ctx,
 					      const struct location *loc,
 					      unsigned int reg,
-					      unsigned int len)
+					      int len)
 {
 	struct expr *concat, *expr;
 
@@ -134,7 +134,7 @@ err:
 static struct expr *netlink_parse_concat_data(struct netlink_parse_ctx *ctx,
 					      const struct location *loc,
 					      unsigned int reg,
-					      unsigned int len,
+					      int len,
 					      struct expr *data)
 {
 	struct expr *concat, *expr, *i;
Signature · 1 line
1.9.1
maintainer
#1 lore raw
Quoted 11 lines · Shivani Bhardwaj

Change the data type of len from unsigned int to int in order to make it valid for checks like

if (len < 0)

The issue was brought into attention by the unexplained behavior of frag with frag-off. Bugzilla entry:
https://bugzilla.netfilter.org/show_bug.cgi?id=935

This patch fixes this bug, however there are still issues with frag that need to be fixed.

exthdr (frag) seems to have several issues:

- we should reject exthdr and only allow it with ipv6.
- for inet/bridge, we should also inject ipv6 dependency
- some exthdrs (frag for instance) have odd bit lengths and need mask/shift instructions.

For example, in your example rule we generate:
[ exthdr load 1b @ 44 + 2 => reg 1 ]
[ cmp eq reg 1 0x00002100 ]

But thats not correct -- we truncated the load to one byte. Instead we should have loaded 2 bytes and then masked off the extra 3bits.

I'll work on this.

#2 lore raw
Quoted 20 lines · Florian Westphal
Quoted 11 lines · Shivani Bhardwaj

Change the data type of len from unsigned int to int in order to make it valid for checks like

if (len < 0)

The issue was brought into attention by the unexplained behavior of frag with frag-off. Bugzilla entry:
https://bugzilla.netfilter.org/show_bug.cgi?id=935

This patch fixes this bug, however there are still issues with frag that need to be fixed.

exthdr (frag) seems to have several issues:

- we should reject exthdr and only allow it with ipv6.
- for inet/bridge, we should also inject ipv6 dependency
- some exthdrs (frag for instance) have odd bit lengths and need mask/shift instructions.

Quoted 8 lines

For example, in your example rule we generate:

   [ exthdr load 1b @ 44 + 2 => reg 1 ]
   [ cmp eq reg 1 0x00002100 ]

But thats not correct -- we truncated the load to one byte. Instead we should have loaded 2 bytes and then masked off the extra 3bits.

I'll work on this.

In the chain this rule shows up as

chain input {
        type filter hook input priority 0; policy accept;
        frag unknown 0x0 [invalid type]
    }

This is also the case with some icmpv6 options (id and max-delay), please take a note of this too.
Thanks.