Patchwork a possible bug in netfilter

login
register
mail settings
Submitter Florian Westphal
Date April 7, 2012, 3:58 p.m.
Message ID <20120407155847.GB28425@Chamillionaire.breakpoint.cc>
Download mbox | patch
Permalink /patch/151307/
State Not Applicable
Headers show

Comments

Florian Westphal - April 7, 2012, 3:58 p.m.
Jan Engelhardt <jengelh@medozas.de> wrote:
> On Friday 2012-04-06 06:12, Serge Leschinsky wrote:
> >
> > Sometimes my system panics and below you can find the information I was able to
> > catch via netconsole.
> >
> > LFS-based system, kernel 3.3.1, xtables-addons 1.41
> >
> > BUG: unable to handle kernel paging request at 00003800 00000000
> > IP: [<ffffffffa02001bd>] xt_psd_match+0x1bd/0x5f4 [xt_psd]
> > 80 00 00 00 00 48 39 f8 0f 84 57 03 00 00 48 89 45 c8
> ><48>8b 00 48 85 c0 75 eb 48 8d 04 92 83 c1 01 89 0d 56 b7 00 00
> 
> Two asm candidates pop up here:
> 
>  168:   48 8b 3f                mov    rdi,QWORD PTR [rdi]
> 
>  372:   48 8b 00                mov    rax,QWORD PTR [rax]
>  375:   48 85 c0                test   rax,rax
> 
> This maps to lines lines similar to  (curr = curr->next) != NULL.

I wonder wheter we're corrupting curr->next via curr->ports[] overflow.

Serge, could you try this patch?

--
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
Serge Leschinsky - April 8, 2012, 4:07 a.m.
On 04/07/2012 08:58 AM, Florian Westphal wrote:
....
> Serge, could you try this patch?
>
> diff --git a/extensions/xt_psd.c b/extensions/xt_psd.c
> index 46b2831..acb5e8e 100644
> --- a/extensions/xt_psd.c
> +++ b/extensions/xt_psd.c
> @@ -227,7 +227,7 @@ xt_psd_match(const struct sk_buff *pskb, struct xt_action_param *match)
>   				goto out_match;
>
>   			/* Remember the new port */
> -			if (curr->count<  SCAN_MAX_COUNT) {
> +			if (curr->count<  ARRAY_SIZE(curr->ports)) {
>   				curr->ports[curr->count].number = dest_port;
>   				curr->ports[curr->count].proto = proto;
>   				curr->ports[curr->count].and_flags = tcp_flags;
>

The patch is applied. I'll monitor logs for about a week to make sure there are 
no more panics.

Thank you!
Serge
--
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
Serge Leschinsky - April 17, 2012, 10:20 p.m.
Florian,

I think the problem is fixed - the boxes work fine, I didn’t notice panics from 
the update.

Thanks a lot!

Serge


On 04/07/2012 09:07 PM, Serge Leschinsky wrote:
> On 04/07/2012 08:58 AM, Florian Westphal wrote:
> ....
>> Serge, could you try this patch?
>>
>> diff --git a/extensions/xt_psd.c b/extensions/xt_psd.c
>> index 46b2831..acb5e8e 100644
>> --- a/extensions/xt_psd.c
>> +++ b/extensions/xt_psd.c
>> @@ -227,7 +227,7 @@ xt_psd_match(const struct sk_buff *pskb, struct
>> xt_action_param *match)
>> goto out_match;
>>
>> /* Remember the new port */
>> - if (curr->count< SCAN_MAX_COUNT) {
>> + if (curr->count< ARRAY_SIZE(curr->ports)) {
>> curr->ports[curr->count].number = dest_port;
>> curr->ports[curr->count].proto = proto;
>> curr->ports[curr->count].and_flags = tcp_flags;
>>
>
> The patch is applied. I'll monitor logs for about a week to make sure there are
> no more panics.
>
> Thank you!
> Serge

--
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

Patch

diff --git a/extensions/xt_psd.c b/extensions/xt_psd.c
index 46b2831..acb5e8e 100644
--- a/extensions/xt_psd.c
+++ b/extensions/xt_psd.c
@@ -227,7 +227,7 @@  xt_psd_match(const struct sk_buff *pskb, struct xt_action_param *match)
 				goto out_match;
 
 			/* Remember the new port */
-			if (curr->count < SCAN_MAX_COUNT) {
+			if (curr->count < ARRAY_SIZE(curr->ports)) {
 				curr->ports[curr->count].number = dest_port;
 				curr->ports[curr->count].proto = proto;
 				curr->ports[curr->count].and_flags = tcp_flags;