Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1642195 > unrolled thread

[PATCH net v2] net: x25: fix one potential use-after-free issue

Back to article view | Back to linux.kernel

Contents

  [PATCH net v2] net: x25: fix one potential use-after-free issue linzhang <xiaolou4617@gmail.com> - 2017-05-16 06:00 +0200
    Re: [PATCH net v2] net: x25: fix one potential use-after-free issue David Miller <davem@davemloft.net> - 2017-05-16 19:20 +0200

#1642195 — [PATCH net v2] net: x25: fix one potential use-after-free issue

The function x25_init is not properly unregister related resources
on error handler.It is will result in kernel oops if x25_init init
failed, so add properly unregister call on error handler.

Also, i adjust the coding style and make x25_register_sysctl properly
return failure.

Signed-off-by: linzhang <xiaolou4617@gmail.com>
---
 include/net/x25.h        |  4 ++--
 net/x25/af_x25.c         | 34 +++++++++++++++++++++-------------
 net/x25/sysctl_net_x25.c |  5 ++++-
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/include/net/x25.h b/include/net/x25.h
index c383aa4..339820c 100644
--- a/include/net/x25.h
+++ b/include/net/x25.h
@@ -298,10 +298,10 @@ int x25_decode(struct sock *, struct sk_buff *, int *, int *, int *, int *,
 
 /* sysctl_net_x25.c */
 #ifdef CONFIG_SYSCTL
-void x25_register_sysctl(void);
+int x25_register_sysctl(void);
 void x25_unregister_sysctl(void);
 #else
-static inline void x25_register_sysctl(void) {};
+static inline int x25_register_sysctl(void) { return 0 };
 static inline void x25_unregister_sysctl(void) {};
 #endif /* CONFIG_SYSCTL */
 
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 8b911c2..75c64de 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1791,34 +1791,42 @@ void x25_kill_by_neigh(struct x25_neigh *nb)
 
 static int __init x25_init(void)
 {
-	int rc = proto_register(&x25_proto, 0);
+	int rc;
 
-	if (rc != 0)
+	rc = proto_register(&x25_proto, 0);
+	if (rc)
 		goto out;
 
 	rc = sock_register(&x25_family_ops);
-	if (rc != 0)
-		goto out_proto;
+	if (rc)
+		goto out_sock;
 
 	dev_add_pack(&x25_packet_type);
 
 	rc = register_netdevice_notifier(&x25_dev_notifier);
-	if (rc != 0)
-		goto out_sock;
+	if (rc)
+		goto out_dev;
 
-	pr_info("Linux Version 0.2\n");
+	rc = x25_register_sysctl();
+	if (rc)
+		goto out_sysctl;
 
-	x25_register_sysctl();
 	rc = x25_proc_init();
-	if (rc != 0)
-		goto out_dev;
+	if (rc)
+		goto out_proc;
+
+	pr_info("Linux Version 0.2\n");
+
 out:
 	return rc;
-out_dev:
+out_proc:
+	x25_unregister_sysctl();
+out_sysctl:
 	unregister_netdevice_notifier(&x25_dev_notifier);
-out_sock:
+out_dev:
+	dev_remove_pack(&x25_packet_type);
 	sock_unregister(AF_X25);
-out_proto:
+out_sock:
 	proto_unregister(&x25_proto);
 	goto out;
 }
diff --git a/net/x25/sysctl_net_x25.c b/net/x25/sysctl_net_x25.c
index a06dfe1..ba078c8 100644
--- a/net/x25/sysctl_net_x25.c
+++ b/net/x25/sysctl_net_x25.c
@@ -73,9 +73,12 @@
 	{ },
 };
 
-void __init x25_register_sysctl(void)
+int __init x25_register_sysctl(void)
 {
 	x25_table_header = register_net_sysctl(&init_net, "net/x25", x25_table);
+	if (!x25_table_header)
+		return -ENOMEM;
+	return 0;
 }
 
 void x25_unregister_sysctl(void)
-- 
1.8.3.1

[toc] | [next] | [standalone]

#1642681

From: linzhang <xiaolou4617@gmail.com>
Date: Tue, 16 May 2017 11:52:54 +0800

>  	return rc;
> -out_dev:
> +out_proc:
> +	x25_unregister_sysctl();
> +out_sysctl:
>  	unregister_netdevice_notifier(&x25_dev_notifier);
> -out_sock:
> +out_dev:
> +	dev_remove_pack(&x25_packet_type);
>  	sock_unregister(AF_X25);
> -out_proto:
> +out_sock:
>  	proto_unregister(&x25_proto);
>  	goto out;
>  }

Please do not name the labels this way, retain the current convention.
And that is the label names what resources get released not the
resource that we failed to acquire.

So the first label, should be "out_sysctl:" because that is what we
are releasing.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web