All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xin Long <lucien.xin@gmail.com>
To: network dev <netdev@vger.kernel.org>
Cc: davem@davemloft.net, Eric Dumazet <edumazet@google.com>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	Sabrina Dubroca <sd@queasysnail.net>
Subject: [PATCH net 1/2] sock_diag: request _diag module only when the family has been registered
Date: Thu, 19 Oct 2017 15:32:24 +0800	[thread overview]
Message-ID: <e36e2f048c38d63f10386798dc1eced0882df9bf.1508398111.git.lucien.xin@gmail.com> (raw)
In-Reply-To: <cover.1508398111.git.lucien.xin@gmail.com>
In-Reply-To: <cover.1508398111.git.lucien.xin@gmail.com>

Now when using 'ss' in iproute, kernel would try to load all _diag
modules. It causes the corresponding family or proto modules to be
loaded as well.

Like after 'ss -a', sctp, dccp, af_packet(if it works as a moudle)
will be loaded.

As these family or proto modules are loaded unexpectly, this might
have some security implications.

This patch is to introduce sock_diag_request_module() in sock_diag
where we only request _diag module when it's corresponding family
has been registered. The fix for inet_diag will be done in later
patch.

Note that we can't just load _diag module without the family or
proto loaded, as some symbols in _diag module are from the family
or proto moudle.

Fixes: 8ef874bfc729 ("sock_diag: Move the sock_ code to net/core/")
Reported-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 include/linux/net.h  |  1 +
 net/core/sock_diag.c | 21 +++++++++++++--------
 net/socket.c         |  5 +++++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index d97d80d..6c7cf09 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -225,6 +225,7 @@ enum {
 int sock_wake_async(struct socket_wq *sk_wq, int how, int band);
 int sock_register(const struct net_proto_family *fam);
 void sock_unregister(int family);
+bool sock_is_registered(int family);
 int __sock_create(struct net *net, int family, int type, int proto,
 		  struct socket **res, int kern);
 int sock_create(int family, int type, int proto, struct socket **res);
diff --git a/net/core/sock_diag.c b/net/core/sock_diag.c
index 217f4e3..643a446 100644
--- a/net/core/sock_diag.c
+++ b/net/core/sock_diag.c
@@ -207,6 +207,15 @@ void sock_diag_unregister(const struct sock_diag_handler *hnld)
 }
 EXPORT_SYMBOL_GPL(sock_diag_unregister);
 
+static int sock_diag_request_module(int family)
+{
+	if (!sock_is_registered(family))
+		return -ENOENT;
+
+	return request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
+			      NETLINK_SOCK_DIAG, family);
+}
+
 static int __sock_diag_cmd(struct sk_buff *skb, struct nlmsghdr *nlh)
 {
 	int err;
@@ -220,8 +229,7 @@ static int __sock_diag_cmd(struct sk_buff *skb, struct nlmsghdr *nlh)
 		return -EINVAL;
 
 	if (sock_diag_handlers[req->sdiag_family] == NULL)
-		request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
-				NETLINK_SOCK_DIAG, req->sdiag_family);
+		sock_diag_request_module(req->sdiag_family);
 
 	mutex_lock(&sock_diag_table_mutex);
 	hndl = sock_diag_handlers[req->sdiag_family];
@@ -247,8 +255,7 @@ static int sock_diag_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
 	case TCPDIAG_GETSOCK:
 	case DCCPDIAG_GETSOCK:
 		if (inet_rcv_compat == NULL)
-			request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
-					NETLINK_SOCK_DIAG, AF_INET);
+			sock_diag_request_module(AF_INET);
 
 		mutex_lock(&sock_diag_table_mutex);
 		if (inet_rcv_compat != NULL)
@@ -281,14 +288,12 @@ static int sock_diag_bind(struct net *net, int group)
 	case SKNLGRP_INET_TCP_DESTROY:
 	case SKNLGRP_INET_UDP_DESTROY:
 		if (!sock_diag_handlers[AF_INET])
-			request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
-				       NETLINK_SOCK_DIAG, AF_INET);
+			sock_diag_request_module(AF_INET);
 		break;
 	case SKNLGRP_INET6_TCP_DESTROY:
 	case SKNLGRP_INET6_UDP_DESTROY:
 		if (!sock_diag_handlers[AF_INET6])
-			request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
-				       NETLINK_SOCK_DIAG, AF_INET);
+			sock_diag_request_module(AF_INET);
 		break;
 	}
 	return 0;
diff --git a/net/socket.c b/net/socket.c
index c729625..733d657 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2590,6 +2590,11 @@ void sock_unregister(int family)
 }
 EXPORT_SYMBOL(sock_unregister);
 
+bool sock_is_registered(int family)
+{
+	return family < NPROTO && rcu_access_pointer(net_families[family]);
+}
+
 static int __init sock_init(void)
 {
 	int err;
-- 
2.1.0

  reply	other threads:[~2017-10-19  7:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19  7:32 [PATCH net 0/2] net: diag: fix a potential security issue Xin Long
2017-10-19  7:32 ` Xin Long [this message]
2017-10-19  7:32   ` [PATCH net 2/2] inet_diag: request _diag module only when the proto has been registered Xin Long
2017-10-21  1:27 ` [PATCH net 0/2] net: diag: fix a potential security issue David Miller
     [not found]   ` <CADvbK_fWmmC3ggpoT--Pxk3GxZ8Gq_rbdFGTuXk-BuTHTO=eXw@mail.gmail.com>
2017-10-21  6:18     ` Eric Dumazet
2017-10-21  6:51       ` Xin Long
2017-10-21  7:45         ` Eric Dumazet
2017-10-21  8:45           ` Xin Long
2017-10-21  9:45             ` Xin Long
2017-10-21 11:16               ` David Miller
2017-10-21 11:14     ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e36e2f048c38d63f10386798dc1eced0882df9bf.1508398111.git.lucien.xin@gmail.com \
    --to=lucien.xin@gmail.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=sd@queasysnail.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.