| Message ID | 20160228194043.GA15021@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
Shivani Bhardwaj <shivanib134@gmail.com> wrote: > 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 29, 2016 at 3:36 PM, Florian Westphal <fw@strlen.de> wrote: > Shivani Bhardwaj <shivanib134@gmail.com> wrote: >> 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. 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. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index ae6abb0..2d7a417 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -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;
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. Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- src/netlink_delinearize.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)