Skip to contentSkip to conversation

ipv4: fix kfree static array pointer in ipv4_sysctl_exit_net

PATCH In review Wang Weidong 2 participants 5 messages May 8, 2014 – May 9, 2014

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Wang Weidong

author
#0 lore raw

PATCH 1/1 ipv4: fix kfree static array pointer in ipv4_sysctl_exit_net

In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which is a static array pointer. So when do ipv4_sysctl_exit_net, it will free the ipv4_net_table for init_net.

So add a check net_namespace init_net before kfree the sysctl_table.

Signed-off-by Wang Weidong <[email protected]>
---
 net/ipv4/sysctl_net_ipv4.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/sysctl_net_ipv4.c +2 -1
index 44eba05..2825577 100644
@@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net)
 
 	table = net->ipv4.ipv4_hdr->ctl_table_arg;
 	unregister_net_sysctl_table(net->ipv4.ipv4_hdr);
-	kfree(table);
+	if (!net_eq(net, &init_net))
+		kfree(table);
 }
 
 static __net_initdata struct pernet_operations ipv4_sysctl_ops = {
Signature · 1 line
1.7.12
#1 lore raw
Quoted 17 lines · Wang Weidong

In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which is a static array pointer. So when do ipv4_sysctl_exit_net, it will free the ipv4_net_table for init_net.

So add a check net_namespace init_net before kfree the sysctl_table.

Signed-off-by Wang Weidong <[email protected]>
---
 net/ipv4/sysctl_net_ipv4.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/sysctl_net_ipv4.c +0 -0
index 44eba05..2825577 100644

net/ipv4/sysctl_net_ipv4.c static __net_exit void ipv4_sysctl_exit_net(struct net *net) · line 898

Quoted 9 lines
net/ipv4/sysctl_net_ipv4.c +2 -1
@@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net)
 
 	table = net->ipv4.ipv4_hdr->ctl_table_arg;
 	unregister_net_sysctl_table(net->ipv4.ipv4_hdr);
-	kfree(table);
+	if (!net_eq(net, &init_net))
+		kfree(table);
 }
 
 static __net_initdata struct pernet_operations ipv4_sysctl_ops = {

Could you explain how you can trigger this case, calling ipv4_sysctl_exit_net() with net == &init_net ?

This would be a bug, your patch would try to hide it maybe ?

author
#2 lore raw
Quoted 26 lines · Eric Dumazet
Quoted 25 lines · Wang Weidong

In ipv4_sysctl_init_net, we don't kmemdup a sysctl_table for init_net, so init_net->ipv4.ipv4_hdr->ctl_table_arg points to ipv4_net_table which is a static array pointer. So when do ipv4_sysctl_exit_net, it will free the ipv4_net_table for init_net.

So add a check net_namespace init_net before kfree the sysctl_table.

Signed-off-by Wang Weidong <[email protected]>
---
 net/ipv4/sysctl_net_ipv4.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/sysctl_net_ipv4.c +2 -1
index 44eba05..2825577 100644
@@ -891,7 +891,8 @@ static __net_exit void ipv4_sysctl_exit_net(struct net *net)
 
 	table = net->ipv4.ipv4_hdr->ctl_table_arg;
 	unregister_net_sysctl_table(net->ipv4.ipv4_hdr);
-	kfree(table);
+	if (!net_eq(net, &init_net))
+		kfree(table);
 }
Quoted 7 lines

net/ipv4/sysctl_net_ipv4.c static __net_exit void ipv4_sysctl_exit_net(struct net *net) · line 898

Quoted 1 line
net/ipv4/sysctl_net_ipv4.c +0 -0
 static __net_initdata struct pernet_operations ipv4_sysctl_ops = {

Could you explain how you can trigger this case, calling ipv4_sysctl_exit_net() with net == &init_net ?

This would be a bug, your patch would try to hide it maybe ?

No.
I just trigger the similar case on sctp when I do 'rmmod -f sctp'. Here I add the init_net case for sctp register sysctl.

Is it better to add BUG_ON(net == &init_net) maybe?

Regards
Wang

Quoted 3 lines
#3 lore raw
Quoted 6 lines

No.
I just trigger the similar case on sctp when I do 'rmmod -f sctp'. Here I add the init_net case for sctp register sysctl.

Is it better to add BUG_ON(net == &init_net) maybe?

The point is : SCTP _can_ be a module, but IPV4 is not.

You cannot rmmod ipv4

Its not because there is a bug in SCTP that you can apply the same recipe on another layer.

author
#4 lore raw
Quoted 7 lines · Eric Dumazet
Quoted 6 lines

No.
I just trigger the similar case on sctp when I do 'rmmod -f sctp'. Here I add the init_net case for sctp register sysctl.

Is it better to add BUG_ON(net == &init_net) maybe?

Quoted 7 lines

The point is : SCTP _can_ be a module, but IPV4 is not.

You cannot rmmod ipv4

Its not because there is a bug in SCTP that you can apply the same recipe on another layer.

Well, You are right. So ignore it.

Regards
Wang

Quoted 1 line