[net-next 0/2] sctp: fix kfree static array pointer in sctp_sysctl_net_unregister

[net-next 0/2] sctp: fix kfree static array pointer in sctp_sysctl_net_unregister

[Wang Weidong]

patch #1 revert the efb842c45("sctp: optimize the sctp_sysctl_net_register")
patch #2 add a checking for sctp_sysctl_net_register

Wang Weidong (2):
  Revert "sctp: optimize the sctp_sysctl_net_register"
  sctp: add a checking for sctp_sysctl_net_register

 net/sctp/sysctl.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

-- 
1.7.12

[net-next 1/2] Revert "sctp: optimize the sctp_sysctl_net_register"

[Wang Weidong]

This revert commit efb842c45("sctp: optimize the sctp_sysctl_net_register"),
Since it doesn't kmemdup a sysctl_table for init_net, so the
init_net->sctp.sysctl_header->ctl_table_arg points to sctp_net_table
which is a static array pointer. So when doing sctp_sysctl_net_unregister,
it will free sctp_net_table, then we will get a NULL pointer dereference
like that:

[  262.948220] BUG: unable to handle kernel NULL pointer dereference at 000000000000006c
[  262.948232] IP: [<ffffffff81144b70>] kfree+0x80/0x420
[  262.948260] PGD db80a067 PUD dae12067 PMD 0
[  262.948268] Oops: 0000 [#1] SMP
[  262.948273] Modules linked in: sctp(-) crc32c_generic libcrc32c
...
[  262.948338] task: ffff8800db830190 ti: ffff8800dad00000 task.ti: ffff8800dad00000
[  262.948344] RIP: 0010:[<ffffffff81144b70>]  [<ffffffff81144b70>] kfree+0x80/0x420
[  262.948353] RSP: 0018:ffff8800dad01d88  EFLAGS: 00010046
[  262.948358] RAX: 0100000000000000 RBX: ffffffffa0227940 RCX: ffffea0000707888
[  262.948363] RDX: ffffea0000707888 RSI: 0000000000000001 RDI: ffffffffa0227940
[  262.948369] RBP: ffff8800dad01de8 R08: 0000000000000000 R09: ffff8800d9e983a9
[  262.948374] R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa0227940
[  262.948380] R13: ffffffff8187cfc0 R14: 0000000000000000 R15: ffffffff8187da10
[  262.948386] FS:  00007fa2a2658700(0000) GS:ffff880112800000(0000) knlGS:0000000000000000
[  262.948394] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  262.948400] CR2: 000000000000006c CR3: 00000000cddc0000 CR4: 00000000000006e0
[  262.948410] Stack:
[  262.948413]  ffff8800dad01da8 0000000000000286 0000000020227940 ffffffffa0227940
[  262.948422]  ffff8800dad01dd8 ffffffff811b7fa1 ffffffffa0227940 ffffffffa0227940
[  262.948431]  ffffffff8187d960 ffffffff8187cfc0 ffffffff8187d960 ffffffff8187da10
[  262.948440] Call Trace:
[  262.948457]  [<ffffffff811b7fa1>] ? unregister_sysctl_table+0x51/0xa0
[  262.948476]  [<ffffffffa020d1a1>] sctp_sysctl_net_unregister+0x21/0x30 [sctp]
[  262.948490]  [<ffffffffa020ef6d>] sctp_net_exit+0x12d/0x150 [sctp]
[  262.948512]  [<ffffffff81394f49>] ops_exit_list+0x39/0x60
[  262.948522]  [<ffffffff813951ed>] unregister_pernet_operations+0x3d/0x70
[  262.948530]  [<ffffffff81395292>] unregister_pernet_subsys+0x22/0x40
[  262.948544]  [<ffffffffa020efcc>] sctp_exit+0x3c/0x12d [sctp]
[  262.948562]  [<ffffffff810c5e04>] SyS_delete_module+0x194/0x210
[  262.948577]  [<ffffffff81240fde>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[  262.948587]  [<ffffffff815217a2>] system_call_fastpath+0x16/0x1b

With this revert, it won't occur the Oops.

Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/sysctl.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index c82fdc1..ee80eb4 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -436,18 +436,15 @@ static int proc_sctp_do_auth(struct ctl_table *ctl, int write,
 
 int sctp_sysctl_net_register(struct net *net)
 {
-	struct ctl_table *table = sctp_net_table;
-
-	if (!net_eq(net, &init_net)) {
-		int i;
+	struct ctl_table *table;
+	int i;
 
-		table = kmemdup(sctp_net_table, sizeof(sctp_net_table), GFP_KERNEL);
-		if (!table)
-			return -ENOMEM;
+	table = kmemdup(sctp_net_table, sizeof(sctp_net_table), GFP_KERNEL);
+	if (!table)
+		return -ENOMEM;
 
-		for (i = 0; table[i].data; i++)
-			table[i].data += (char *)(&net->sctp) - (char *)&init_net.sctp;
-	}
+	for (i = 0; table[i].data; i++)
+		table[i].data += (char *)(&net->sctp) - (char *)&init_net.sctp;
 
 	net->sctp.sysctl_header = register_net_sysctl(net, "net/sctp", table);
 	return 0;
-- 
1.7.12

[net-next 2/2] sctp: add a checking for sctp_sysctl_net_register

[Wang Weidong]

When register_net_sysctl failed, we should free the
sysctl_table.

Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
 net/sctp/sysctl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
index ee80eb4..7e5eb75 100644
--- a/net/sctp/sysctl.c
+++ b/net/sctp/sysctl.c
@@ -447,6 +447,10 @@ int sctp_sysctl_net_register(struct net *net)
 		table[i].data += (char *)(&net->sctp) - (char *)&init_net.sctp;
 
 	net->sctp.sysctl_header = register_net_sysctl(net, "net/sctp", table);
+	if (net->sctp.sysctl_header == NULL) {
+		kfree(table);
+		return -ENOMEM;
+	}
 	return 0;
 }
 
-- 
1.7.12

Re: [net-next 0/2] sctp: fix kfree static array pointer in sctp_sysctl_net_unregister

[Neil Horman]

On Thu, May 08, 2014 at 08:55:00PM +0800, Wang Weidong wrote:
> patch #1 revert the efb842c45("sctp: optimize the sctp_sysctl_net_register")
> patch #2 add a checking for sctp_sysctl_net_register
> 
> Wang Weidong (2):
>   Revert "sctp: optimize the sctp_sysctl_net_register"
>   sctp: add a checking for sctp_sysctl_net_register
> 
>  net/sctp/sysctl.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> -- 
> 1.7.12
> 
> 
> 

Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [net-next 0/2] sctp: fix kfree static array pointer in sctp_sysctl_net_unregister

[David Miller]

From: Wang Weidong <wangweidong1@huawei.com>
Date: Thu, 8 May 2014 20:55:00 +0800

> patch #1 revert the efb842c45("sctp: optimize the sctp_sysctl_net_register")
> patch #2 add a checking for sctp_sysctl_net_register

Series applied, thanks.