[PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Joonwoo Park]

icase of xt_string_info indicates case [in]sensitive matching.
netfilter can find cmd.exe, Cmd.exe, cMd.exe and etc easily.

Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
---
 include/linux/netfilter/xt_string.h |    1 +
 net/netfilter/xt_string.c           |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/xt_string.h b/include/linux/netfilter/xt_string.h
index bb21dd1..dfd347f 100644
--- a/include/linux/netfilter/xt_string.h
+++ b/include/linux/netfilter/xt_string.h
@@ -12,6 +12,7 @@ struct xt_string_info
 	char 	  pattern[XT_STRING_MAX_PATTERN_SIZE];
 	u_int8_t  patlen;
 	u_int8_t  invert;
+	u_int8_t  icase;
 
 	/* Used internally by the kernel */
 	struct ts_config __attribute__((aligned(8))) *config;
diff --git a/net/netfilter/xt_string.c b/net/netfilter/xt_string.c
index 9508484..7c83431 100644
--- a/net/netfilter/xt_string.c
+++ b/net/netfilter/xt_string.c
@@ -55,7 +55,7 @@ string_mt_check(const char *tablename, const void *ip,
 	if (conf->patlen > XT_STRING_MAX_PATTERN_SIZE)
 		return false;
 	ts_conf = textsearch_prepare(conf->algo, conf->pattern, conf->patlen,
-				     0, GFP_KERNEL, TS_AUTOLOAD);
+				     conf->icase, GFP_KERNEL, TS_AUTOLOAD);
 	if (IS_ERR(ts_conf))
 		return false;
 
-- 
1.5.4.3

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Patrick McHardy]

Joonwoo Park wrote:
> icase of xt_string_info indicates case [in]sensitive matching.
> netfilter can find cmd.exe, Cmd.exe, cMd.exe and etc easily.
> 
> Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
> ---
>  include/linux/netfilter/xt_string.h |    1 +
>  net/netfilter/xt_string.c           |    2 +-
>  2 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/netfilter/xt_string.h b/include/linux/netfilter/xt_string.h
> index bb21dd1..dfd347f 100644
> --- a/include/linux/netfilter/xt_string.h
> +++ b/include/linux/netfilter/xt_string.h
> @@ -12,6 +12,7 @@ struct xt_string_info
>  	char 	  pattern[XT_STRING_MAX_PATTERN_SIZE];
>  	u_int8_t  patlen;
>  	u_int8_t  invert;
> +	u_int8_t  icase;
>  
>  	/* Used internally by the kernel */
>  	struct ts_config __attribute__((aligned(8))) *config;


A few words for the changelog why this won't break compatibility
by changing the data structure would be nice, especially
considering CRIS that doesn't perform any padding itself (you
might be lucky though because of aligned attribute, but please
verify that yourself).

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Jan Engelhardt]

On Saturday 2008-06-21 09:54, Joonwoo Park wrote:

>icase of xt_string_info indicates case [in]sensitive matching.
>netfilter can find cmd.exe, Cmd.exe, cMd.exe and etc easily.
>
>Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
>---
> include/linux/netfilter/xt_string.h |    1 +
> net/netfilter/xt_string.c           |    2 +-
> 2 files changed, 2 insertions(+), 1 deletions(-)
>
>diff --git a/include/linux/netfilter/xt_string.h b/include/linux/netfilter/xt_string.h
>index bb21dd1..dfd347f 100644
>--- a/include/linux/netfilter/xt_string.h
>+++ b/include/linux/netfilter/xt_string.h
>@@ -12,6 +12,7 @@ struct xt_string_info
> 	char 	  pattern[XT_STRING_MAX_PATTERN_SIZE];
> 	u_int8_t  patlen;
> 	u_int8_t  invert;
>+	u_int8_t  icase;
> 
> 	/* Used internally by the kernel */
> 	struct ts_config __attribute__((aligned(8))) *config;

Why not just doing it this way?


enum {
	XT_STRING_INVERT = 1 << 0,
	XT_STRING_ICASE  = 1 << 1,
};

struct xt_string_info {
	...
	union {
		uint8_t invert;
		uint8_t flags;
	};
	...
};

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Joonwoo Park]

2008/6/29 Jan Engelhardt <jengelh@medozas.de>:
>
> On Saturday 2008-06-21 09:54, Joonwoo Park wrote:
>
>>icase of xt_string_info indicates case [in]sensitive matching.
>>netfilter can find cmd.exe, Cmd.exe, cMd.exe and etc easily.
>>
>>Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
>>---
>> include/linux/netfilter/xt_string.h |    1 +
>> net/netfilter/xt_string.c           |    2 +-
>> 2 files changed, 2 insertions(+), 1 deletions(-)
>>
>>diff --git a/include/linux/netfilter/xt_string.h b/include/linux/netfilter/xt_string.h
>>index bb21dd1..dfd347f 100644
>>--- a/include/linux/netfilter/xt_string.h
>>+++ b/include/linux/netfilter/xt_string.h
>>@@ -12,6 +12,7 @@ struct xt_string_info
>>       char      pattern[XT_STRING_MAX_PATTERN_SIZE];
>>       u_int8_t  patlen;
>>       u_int8_t  invert;
>>+      u_int8_t  icase;
>>
>>       /* Used internally by the kernel */
>>       struct ts_config __attribute__((aligned(8))) *config;
>
> Why not just doing it this way?
>
>
> enum {
>        XT_STRING_INVERT = 1 << 0,
>        XT_STRING_ICASE  = 1 << 1,
> };
>
> struct xt_string_info {
>        ...
>        union {
>                uint8_t invert;
>                uint8_t flags;
>        };
>        ...
> };
>
>

Not bad for me,
I think invert might be removed and flags can contain XT_STRING_INVERT as well.
But I guess we should do version checking between user-space and
kernel more strictly whatever.

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Jan Engelhardt]

On Monday 2008-06-30 20:44, Joonwoo Park wrote:
>> Why not just doing it this way?
>>
>>
>> enum {
>>        XT_STRING_INVERT = 1 << 0,
>>        XT_STRING_ICASE  = 1 << 1,
>> };
>>
>> struct xt_string_info {
>>        ...
>>        union {
>>                uint8_t invert;
>>                uint8_t flags;
>>        };
>>        ...
>> };
>>
>>
>
>Not bad for me,
>I think invert might be removed and flags can contain XT_STRING_INVERT as well.

Right, since we copy eader files anyway, we can just make it uint8_t flags
without the union.

>But I guess we should do version checking between user-space and
>kernel more strictly whatever.

That is what we have '.revision' in xt_match/xt_target for.
In this case however it should be fine with just flags.

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Joonwoo Park]

2008/6/30 Jan Engelhardt <jengelh@medozas.de>:
> Right, since we copy eader files anyway, we can just make it uint8_t flags
> without the union.

I like it :-)
Patrick, Thomas, If you want I'll send this fix with version checking patch.

>
>>But I guess we should do version checking between user-space and
>>kernel more strictly whatever.
>
> That is what we have '.revision' in xt_match/xt_target for.
> In this case however it should be fine with just flags.
>

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Patrick McHardy]

Joonwoo Park wrote:
> 2008/6/30 Jan Engelhardt <jengelh@medozas.de>:
>> Right, since we copy eader files anyway, we can just make it uint8_t flags
>> without the union.
> 
> I like it :-)
> Patrick, Thomas, If you want I'll send this fix with version checking patch.

Yes, please also include the suggestions I made previously
that still apply (should be all of them).

Re: [PATCH 6/8] netfilter: fix string extension for case insensitive pattern matching

[Joonwoo Park]

2008/6/30 Patrick McHardy <kaber@trash.net>:
> Joonwoo Park wrote:
>>
>> 2008/6/30 Jan Engelhardt <jengelh@medozas.de>:
>>>
>>> Right, since we copy eader files anyway, we can just make it uint8_t
>>> flags
>>> without the union.
>>
>> I like it :-)
>> Patrick, Thomas, If you want I'll send this fix with version checking
>> patch.
>
> Yes, please also include the suggestions I made previously
> that still apply (should be all of them).
>

ok, as soon as I finish travel, I'll catch up your suggestions, thanks!