Re: [netfilter-core] [PATCH] no __initdata in netfilter?
From: Harald Welte
Date: Tue Dec 14 2004 - 08:04:19 EST
On Sun, Nov 14, 2004 at 12:26:10PM +0100, Andries Brouwer wrote:
> On Sun, Nov 14, 2004 at 08:56:29AM +0100, Patrick McHardy wrote:
> > Andries Brouwer wrote:
> >
> > >Stuff marked initdata that is referenced in non-init context.
> >
> > Where ? The initial tables are replaced by ipt_register_table.
>
> ip_nat_rule.c:nat_initial_table was referenced in
>
> static struct ipt_table nat_table = {
> .table = &nat_initial_table.repl,
> ...
>
> iptable_filter.c:initial_table was referenced in
>
> static struct ipt_table packet_filter = {
> .table = &initial_table.repl,
> ...
>
> This is not to say that there is a bug here, that the .init
> data would actually be referenced by non-init stuff, but
> it is better to convince oneself by static inspection of
> the binary than by reasoning about the flow of the program.
>
> Where the memory savings are important, the code should
> be rewritten a bit.
We just had a discussion, and the netfilter core team really disagrees
with this change, which apparently was merged in
http://linux.bkbits.net:8080/linux-2.5/cset@xxxxxxxxxxx
I think it's not a good idea to waste memory by removing __initrdata,
just to make it more convenient for some static inspection tools. This
is just the wrong way around.
If we _know_ that it works, and there is no bug, we could just add a
comment "This is handled correctly, since the ip_tables core copies the
data just as rulesets comming from userspace."
Pleaes pull out that change again and submit one that adds a comment, or
alternatively pick up the (incremental) change attached to this mail. I
hope this makes your checker not spit any warnings.
Thanks.
> Andries
--
- Harald Welte <laforge@xxxxxxxxxxxxx> http://www.netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[NETFILTER] re-introduce __initdata to {arp,ip,ip6}_tables
Instead of just removing the (correct) __initdata as introduced by
http://linux.bkbits.net:8080/linux-2.5/cset@xxxxxxxxxxx
we rework the code in order to not trigger some misinterpretation by
static code checkers.
Signed-off-by: Harald Welte <laforge@xxxxxxxxxxxxx>
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/include/linux/netfilter_arp/arp_tables.h linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_arp/arp_tables.h
--- linux-2.6.10-rc3-bk7/include/linux/netfilter_arp/arp_tables.h 2004-10-18 23:53:45.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_arp/arp_tables.h 2004-12-14 12:32:06.488491120 +0100
@@ -312,9 +312,6 @@
/* A unique name... */
char name[ARPT_TABLE_MAXNAMELEN];
- /* Seed table: copied in register_table */
- struct arpt_replace *table;
-
/* What hooks you will enter on */
unsigned int valid_hooks;
@@ -328,7 +325,8 @@
struct module *me;
};
-extern int arpt_register_table(struct arpt_table *table);
+extern int arpt_register_table(struct arpt_table *table,
+ const struct arpt_replace *repl);
extern void arpt_unregister_table(struct arpt_table *table);
extern unsigned int arpt_do_table(struct sk_buff **pskb,
unsigned int hook,
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/include/linux/netfilter_ipv4/ip_tables.h linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_ipv4/ip_tables.h
--- linux-2.6.10-rc3-bk7/include/linux/netfilter_ipv4/ip_tables.h 2004-12-14 11:29:18.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_ipv4/ip_tables.h 2004-12-14 12:26:43.272627400 +0100
@@ -425,9 +425,6 @@
/* A unique name... */
char name[IPT_TABLE_MAXNAMELEN];
- /* Seed table: copied in register_table */
- struct ipt_replace *table;
-
/* What hooks you will enter on */
unsigned int valid_hooks;
@@ -441,7 +438,8 @@
struct module *me;
};
-extern int ipt_register_table(struct ipt_table *table);
+extern int ipt_register_table(struct ipt_table *table,
+ const struct ipt_replace *repl);
extern void ipt_unregister_table(struct ipt_table *table);
extern unsigned int ipt_do_table(struct sk_buff **pskb,
unsigned int hook,
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/include/linux/netfilter_ipv6/ip6_tables.h linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_ipv6/ip6_tables.h
--- linux-2.6.10-rc3-bk7/include/linux/netfilter_ipv6/ip6_tables.h 2004-12-14 11:29:18.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/include/linux/netfilter_ipv6/ip6_tables.h 2004-12-14 12:33:57.966543880 +0100
@@ -429,9 +429,6 @@
/* A unique name... */
char name[IP6T_TABLE_MAXNAMELEN];
- /* Seed table: copied in register_table */
- struct ip6t_replace *table;
-
/* What hooks you will enter on */
unsigned int valid_hooks;
@@ -445,7 +442,8 @@
struct module *me;
};
-extern int ip6t_register_table(struct ip6t_table *table);
+extern int ip6t_register_table(struct ip6t_table *table,
+ const struct ip6t_replace *repl);
extern void ip6t_unregister_table(struct ip6t_table *table);
extern unsigned int ip6t_do_table(struct sk_buff **pskb,
unsigned int hook,
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/arp_tables.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/arp_tables.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/arp_tables.c 2004-12-14 11:29:33.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/arp_tables.c 2004-12-14 12:23:25.900632504 +0100
@@ -1150,7 +1150,8 @@
up(&arpt_mutex);
}
-int arpt_register_table(struct arpt_table *table)
+int arpt_register_table(struct arpt_table *table,
+ const struct arpt_replace *repl)
{
int ret;
struct arpt_table_info *newinfo;
@@ -1158,18 +1159,18 @@
= { 0, 0, 0, { 0 }, { 0 }, { } };
newinfo = vmalloc(sizeof(struct arpt_table_info)
- + SMP_ALIGN(table->table->size) * NR_CPUS);
+ + SMP_ALIGN(repl->size) * NR_CPUS);
if (!newinfo) {
ret = -ENOMEM;
return ret;
}
- memcpy(newinfo->entries, table->table->entries, table->table->size);
+ memcpy(newinfo->entries, repl->entries, repl->size);
ret = translate_table(table->name, table->valid_hooks,
- newinfo, table->table->size,
- table->table->num_entries,
- table->table->hook_entry,
- table->table->underflow);
+ newinfo, repl->size,
+ repl->num_entries,
+ repl->hook_entry,
+ repl->underflow);
duprintf("arpt_register_table: translate table gives %d\n", ret);
if (ret != 0) {
vfree(newinfo);
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/arptable_filter.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/arptable_filter.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/arptable_filter.c 2004-10-18 23:53:06.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/arptable_filter.c 2004-12-14 12:24:04.177813488 +0100
@@ -141,7 +141,6 @@
static struct arpt_table packet_filter = {
.name = "filter",
- .table = &initial_table.repl,
.valid_hooks = FILTER_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.private = NULL,
@@ -184,7 +183,7 @@
int ret, i;
/* Register table */
- ret = arpt_register_table(&packet_filter);
+ ret = arpt_register_table(&packet_filter, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/ip_nat_rule.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/ip_nat_rule.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/ip_nat_rule.c 2004-12-14 11:29:33.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/ip_nat_rule.c 2004-12-14 12:30:29.584222808 +0100
@@ -59,8 +59,8 @@
struct ipt_replace repl;
struct ipt_standard entries[3];
struct ipt_error term;
-} nat_initial_table = {
- { "nat", NAT_VALID_HOOKS, 4,
+} nat_initial_table __initdata
+= { { "nat", NAT_VALID_HOOKS, 4,
sizeof(struct ipt_standard) * 3 + sizeof(struct ipt_error),
{ [NF_IP_PRE_ROUTING] = 0,
[NF_IP_POST_ROUTING] = sizeof(struct ipt_standard),
@@ -110,7 +110,6 @@
static struct ipt_table nat_table = {
.name = "nat",
- .table = &nat_initial_table.repl,
.valid_hooks = NAT_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE,
@@ -298,7 +297,7 @@
{
int ret;
- ret = ipt_register_table(&nat_table);
+ ret = ipt_register_table(&nat_table, &nat_initial_table.repl);
if (ret != 0)
return ret;
ret = ipt_register_target(&ipt_snat_reg);
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/ip_tables.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/ip_tables.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/ip_tables.c 2004-12-14 11:29:33.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/ip_tables.c 2004-12-14 12:16:39.799369280 +0100
@@ -1383,7 +1383,7 @@
up(&ipt_mutex);
}
-int ipt_register_table(struct ipt_table *table)
+int ipt_register_table(struct ipt_table *table, const struct ipt_replace *repl)
{
int ret;
struct ipt_table_info *newinfo;
@@ -1391,17 +1391,17 @@
= { 0, 0, 0, { 0 }, { 0 }, { } };
newinfo = vmalloc(sizeof(struct ipt_table_info)
- + SMP_ALIGN(table->table->size) * NR_CPUS);
+ + SMP_ALIGN(repl->size) * NR_CPUS);
if (!newinfo)
return -ENOMEM;
- memcpy(newinfo->entries, table->table->entries, table->table->size);
+ memcpy(newinfo->entries, repl->entries, repl->size);
ret = translate_table(table->name, table->valid_hooks,
- newinfo, table->table->size,
- table->table->num_entries,
- table->table->hook_entry,
- table->table->underflow);
+ newinfo, repl->size,
+ repl->num_entries,
+ repl->hook_entry,
+ repl->underflow);
if (ret != 0) {
vfree(newinfo);
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_filter.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_filter.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_filter.c 2004-12-14 11:29:33.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_filter.c 2004-12-14 12:30:58.151879864 +0100
@@ -44,8 +44,8 @@
struct ipt_replace repl;
struct ipt_standard entries[3];
struct ipt_error term;
-} initial_table = {
- { "filter", FILTER_VALID_HOOKS, 4,
+} initial_table __initdata
+= { { "filter", FILTER_VALID_HOOKS, 4,
sizeof(struct ipt_standard) * 3 + sizeof(struct ipt_error),
{ [NF_IP_LOCAL_IN] = 0,
[NF_IP_FORWARD] = sizeof(struct ipt_standard),
@@ -95,7 +95,6 @@
static struct ipt_table packet_filter = {
.name = "filter",
- .table = &initial_table.repl,
.valid_hooks = FILTER_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE
@@ -171,7 +170,7 @@
initial_table.entries[1].target.verdict = -forward - 1;
/* Register table */
- ret = ipt_register_table(&packet_filter);
+ ret = ipt_register_table(&packet_filter, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_mangle.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_mangle.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_mangle.c 2004-10-18 23:55:06.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_mangle.c 2004-12-14 12:18:04.940425872 +0100
@@ -125,7 +125,6 @@
static struct ipt_table packet_mangler = {
.name = "mangle",
- .table = &initial_table.repl,
.valid_hooks = MANGLE_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE,
@@ -225,7 +224,7 @@
int ret;
/* Register table */
- ret = ipt_register_table(&packet_mangler);
+ ret = ipt_register_table(&packet_mangler, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_raw.c linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_raw.c
--- linux-2.6.10-rc3-bk7/net/ipv4/netfilter/iptable_raw.c 2004-10-18 23:55:07.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv4/netfilter/iptable_raw.c 2004-12-14 12:18:34.713899616 +0100
@@ -100,7 +100,6 @@
static struct ipt_table packet_raw = {
.name = "raw",
- .table = &initial_table.repl,
.valid_hooks = RAW_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE
@@ -138,7 +137,7 @@
int ret;
/* Register table */
- ret = ipt_register_table(&packet_raw);
+ ret = ipt_register_table(&packet_raw, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6_tables.c linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6_tables.c
--- linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6_tables.c 2004-12-14 11:29:34.000000000 +0100
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6_tables.c 2004-12-14 12:20:42.441482072 +0100
@@ -1463,7 +1463,8 @@
up(&ip6t_mutex);
}
-int ip6t_register_table(struct ip6t_table *table)
+int ip6t_register_table(struct ip6t_table *table,
+ const struct ip6t_replace *repl)
{
int ret;
struct ip6t_table_info *newinfo;
@@ -1471,17 +1472,17 @@
= { 0, 0, 0, { 0 }, { 0 }, { } };
newinfo = vmalloc(sizeof(struct ip6t_table_info)
- + SMP_ALIGN(table->table->size) * NR_CPUS);
+ + SMP_ALIGN(repl->size) * NR_CPUS);
if (!newinfo)
return -ENOMEM;
- memcpy(newinfo->entries, table->table->entries, table->table->size);
+ memcpy(newinfo->entries, repl->entries, repl->size);
ret = translate_table(table->name, table->valid_hooks,
- newinfo, table->table->size,
- table->table->num_entries,
- table->table->hook_entry,
- table->table->underflow);
+ newinfo, repl->size,
+ repl->num_entries,
+ repl->hook_entry,
+ repl->underflow);
if (ret != 0) {
vfree(newinfo);
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_filter.c linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_filter.c
--- linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_filter.c 2004-10-18 23:53:11.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_filter.c 2004-12-14 12:21:11.749026648 +0100
@@ -94,7 +94,6 @@
static struct ip6t_table packet_filter = {
.name = "filter",
- .table = &initial_table.repl,
.valid_hooks = FILTER_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE,
@@ -172,7 +171,7 @@
initial_table.entries[1].target.verdict = -forward - 1;
/* Register table */
- ret = ip6t_register_table(&packet_filter);
+ ret = ip6t_register_table(&packet_filter, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_mangle.c linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_mangle.c
--- linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_mangle.c 2004-10-18 23:53:51.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_mangle.c 2004-12-14 12:22:18.477882320 +0100
@@ -124,7 +124,6 @@
static struct ip6t_table packet_mangler = {
.name = "mangle",
- .table = &initial_table.repl,
.valid_hooks = MANGLE_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE,
@@ -237,7 +236,7 @@
int ret;
/* Register table */
- ret = ip6t_register_table(&packet_mangler);
+ ret = ip6t_register_table(&packet_mangler, &initial_table.repl);
if (ret < 0)
return ret;
diff -Nru --exclude-from /space/home/laforge/scripts/dontdiff --exclude .depend --exclude '*.o' --exclude '*.ko' --exclude '*.ver' --exclude '.*.flags' --exclude '*.orig' --exclude '*.rej' --exclude '*.cmd' --exclude '*.mod.c' --exclude '*~' linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_raw.c linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_raw.c
--- linux-2.6.10-rc3-bk7/net/ipv6/netfilter/ip6table_raw.c 2004-10-18 23:54:39.000000000 +0200
+++ linux-2.6.10-rc3-bk7-initdata/net/ipv6/netfilter/ip6table_raw.c 2004-12-14 12:21:51.191030552 +0100
@@ -108,7 +108,6 @@
static struct ip6t_table packet_raw = {
.name = "raw",
- .table = &initial_table.repl,
.valid_hooks = RAW_VALID_HOOKS,
.lock = RW_LOCK_UNLOCKED,
.me = THIS_MODULE
@@ -145,7 +144,7 @@
int ret;
/* Register table */
- ret = ip6t_register_table(&packet_raw);
+ ret = ip6t_register_table(&packet_raw, &initial_table.repl);
if (ret < 0)
return ret;
Attachment:
signature.asc
Description: Digital signature