Patchwork netfilter: don't xt_jumpstack_alloc twice in xt_register_table

login
register
mail settings
Submitter Xiaotian Feng
Date May 31, 2010, 11:06 a.m.
Message ID <1275303998-2435-1-git-send-email-dfeng@redhat.com>
Download mbox | patch
Permalink /patch/54081/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Xiaotian Feng - May 31, 2010, 11:06 a.m.
In xt_register_table, xt_jumpstack_alloc is called first, later
xt_replace_table is used. But in xt_replace_table, xt_jumpstack_alloc
will be used again. Then the memory allocated by previous xt_jumpstack_alloc
will be leaked. We can simply remove the previous xt_jumpstack_alloc because
there aren't any users of newinfo between xt_jumpstack_alloc and
xt_replace_table.

Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
Cc: Patrick McHardy <kaber@trash.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jan Engelhardt <jengelh@medozas.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
---
 net/netfilter/x_tables.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
Jan Engelhardt - May 31, 2010, 11:51 a.m.
On Monday 2010-05-31 13:06, Xiaotian Feng wrote:

>In xt_register_table, xt_jumpstack_alloc is called first, later
>xt_replace_table is used. But in xt_replace_table, xt_jumpstack_alloc
>will be used again. Then the memory allocated by previous xt_jumpstack_alloc
>will be leaked. We can simply remove the previous xt_jumpstack_alloc because
>there aren't any users of newinfo between xt_jumpstack_alloc and
>xt_replace_table.

Indeed that seems to be so.

>diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
>index 445de70..47b1e79 100644
>--- a/net/netfilter/x_tables.c
>+++ b/net/netfilter/x_tables.c
>@@ -844,10 +844,6 @@ struct xt_table *xt_register_table(struct net *net,
> 	struct xt_table_info *private;
> 	struct xt_table *t, *table;
> 
>-	ret = xt_jumpstack_alloc(newinfo);
>-	if (ret < 0)
>-		return ERR_PTR(ret);
>-
> 	/* Don't add one object to multiple lists. */
> 	table = kmemdup(input_table, sizeof(struct xt_table), GFP_KERNEL);
> 	if (!table) {
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Engelhardt - May 31, 2010, 1:19 p.m.
On Monday 2010-05-31 13:51, Jan Engelhardt wrote:
>On Monday 2010-05-31 13:06, Xiaotian Feng wrote:
>
>>In xt_register_table, xt_jumpstack_alloc is called first, later
>>xt_replace_table is used. But in xt_replace_table, xt_jumpstack_alloc
>>will be used again. Then the memory allocated by previous xt_jumpstack_alloc
>>will be leaked. We can simply remove the previous xt_jumpstack_alloc because
>>there aren't any users of newinfo between xt_jumpstack_alloc and
>>xt_replace_table.
>
>Indeed that seems to be so.

Acked-By: Jan Engelhardt <jengelh@medozas.de>

>
>>diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
>>index 445de70..47b1e79 100644
>>--- a/net/netfilter/x_tables.c
>>+++ b/net/netfilter/x_tables.c
>>@@ -844,10 +844,6 @@ struct xt_table *xt_register_table(struct net *net,
>> 	struct xt_table_info *private;
>> 	struct xt_table *t, *table;
>> 
>>-	ret = xt_jumpstack_alloc(newinfo);
>>-	if (ret < 0)
>>-		return ERR_PTR(ret);
>>-
>> 	/* Don't add one object to multiple lists. */
>> 	table = kmemdup(input_table, sizeof(struct xt_table), GFP_KERNEL);
>> 	if (!table) {
>--
>To unsubscribe from this list: send the line "unsubscribe netfilter" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy - May 31, 2010, 2:34 p.m.
Jan Engelhardt wrote:
> On Monday 2010-05-31 13:51, Jan Engelhardt wrote:
>> On Monday 2010-05-31 13:06, Xiaotian Feng wrote:
>>
>>> In xt_register_table, xt_jumpstack_alloc is called first, later
>>> xt_replace_table is used. But in xt_replace_table, xt_jumpstack_alloc
>>> will be used again. Then the memory allocated by previous xt_jumpstack_alloc
>>> will be leaked. We can simply remove the previous xt_jumpstack_alloc because
>>> there aren't any users of newinfo between xt_jumpstack_alloc and
>>> xt_replace_table.
>> Indeed that seems to be so.
> 
> Acked-By: Jan Engelhardt <jengelh@medozas.de>

Applied, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Patrick McHardy - May 31, 2010, 2:37 p.m.
Jan Engelhardt wrote:
> On Monday 2010-05-31 13:51, Jan Engelhardt wrote:
>> On Monday 2010-05-31 13:06, Xiaotian Feng wrote:
>>
>>> In xt_register_table, xt_jumpstack_alloc is called first, later
>>> xt_replace_table is used. But in xt_replace_table, xt_jumpstack_alloc
>>> will be used again. Then the memory allocated by previous xt_jumpstack_alloc
>>> will be leaked. We can simply remove the previous xt_jumpstack_alloc because
>>> there aren't any users of newinfo between xt_jumpstack_alloc and
>>> xt_replace_table.
>> Indeed that seems to be so.
> 
> Acked-By: Jan Engelhardt <jengelh@medozas.de>

Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" 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/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index 445de70..47b1e79 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -844,10 +844,6 @@  struct xt_table *xt_register_table(struct net *net,
 	struct xt_table_info *private;
 	struct xt_table *t, *table;
 
-	ret = xt_jumpstack_alloc(newinfo);
-	if (ret < 0)
-		return ERR_PTR(ret);
-
 	/* Don't add one object to multiple lists. */
 	table = kmemdup(input_table, sizeof(struct xt_table), GFP_KERNEL);
 	if (!table) {