tipc: add stricter control of reserved service types

TIPC reserves 64 service types for current and future internal use.
Therefore, the bind() function is meant to block regular user sockets
from being bound to these values, while it should let through such
bindings from internal users.

However, since we at the design moment saw no way to distinguish
between regular and internal users the filter function ended up
with allowing all bindings of the reserved types which were really
in use ([0,1]), and block all the rest ([2,63]).

This is risky, since a regular user may bind to the service type
representing the topology server (TIPC_TOP_SRV == 1) or the one used
for indicating neighboring node status (TIPC_CFG_SRV == 0), and wreak
havoc for users of those services, i.e., most users.

The reality is however that TIPC_CFG_SRV never is bound through the
bind() function, since it doesn't represent a regular socket, and
TIPC_TOP_SRV can also be made to bypass the checks in tipc_bind()
by introducing a different entry function, tipc_sk_bind().

It should be noted that although this is a change of the API semantics,
there is no risk we will break any currently working applications by
doing this. Any application trying to bind to the values in question
would be badly broken from the outset, so there is no chance we would
find any such applications in real-world production systems.

v2: Added warning printout when a user is blocked from binding,
    as suggested by Jakub Kicinski

Acked-by: Yung Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Link: https://lore.kernel.org/r/20201030012938.489557-1-jmaloy@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index e795a8a2955b52e27f528715c7844a586b8029a9..69c4b16e8184c14df06a9b4693e57a58ea7b4e9b 100644 (file)
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -658,8 +658,8 @@ static int tipc_release(struct socket *sock)
  * NOTE: This routine doesn't need to take the socket lock since it doesn't
  *       access any non-constant socket information.
  */
-static int tipc_bind(struct socket *sock, struct sockaddr *uaddr,
-                    int uaddr_len)
+
+int tipc_sk_bind(struct socket *sock, struct sockaddr *uaddr, int uaddr_len)
 {
        struct sock *sk = sock->sk;
        struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr;
@@ -691,13 +691,6 @@ static int tipc_bind(struct socket *sock, struct sockaddr *uaddr,
                goto exit;
        }
 
-       if ((addr->addr.nameseq.type < TIPC_RESERVED_TYPES) &&
-           (addr->addr.nameseq.type != TIPC_TOP_SRV) &&
-           (addr->addr.nameseq.type != TIPC_CFG_SRV)) {
-               res = -EACCES;
-               goto exit;
-       }
-
        res = (addr->scope >= 0) ?
                tipc_sk_publish(tsk, addr->scope, &addr->addr.nameseq) :
                tipc_sk_withdraw(tsk, -addr->scope, &addr->addr.nameseq);
@@ -706,6 +699,22 @@ exit:
        return res;
 }
 
+static int tipc_bind(struct socket *sock, struct sockaddr *skaddr, int alen)
+{
+       struct sockaddr_tipc *addr = (struct sockaddr_tipc *)skaddr;
+
+       if (alen) {
+               if (alen < sizeof(struct sockaddr_tipc))
+                       return -EINVAL;
+               if (addr->addr.nameseq.type < TIPC_RESERVED_TYPES) {
+                       pr_warn_once("Can't bind to reserved service type %u\n",
+                                    addr->addr.nameseq.type);
+                       return -EACCES;
+               }
+       }
+       return tipc_sk_bind(sock, skaddr, alen);
+}
+
 /**
  * tipc_getname - get port ID of socket or peer socket
  * @sock: socket structure

diff --git a/net/tipc/socket.h b/net/tipc/socket.h
index b11575afc66fe645003c6ca2475c4e3b874355de..02cdf166807d4b4017a75316e93384d4f68b4cf0 100644 (file)
--- a/net/tipc/socket.h
+++ b/net/tipc/socket.h
@@ -74,7 +74,7 @@ int tipc_dump_done(struct netlink_callback *cb);
 u32 tipc_sock_get_portid(struct sock *sk);
 bool tipc_sk_overlimit1(struct sock *sk, struct sk_buff *skb);
 bool tipc_sk_overlimit2(struct sock *sk, struct sk_buff *skb);
-
+int tipc_sk_bind(struct socket *sock, struct sockaddr *skaddr, int alen);
 int tsk_set_importance(struct sock *sk, int imp);
 
 #endif

diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c
index 5f6f86051c83011f623731b2c9207b7a35781ec8..cec029349662adf5fcaa9b4499eb66b623cb1dde 100644 (file)
--- a/net/tipc/topsrv.c
+++ b/net/tipc/topsrv.c
@@ -520,12 +520,12 @@ static int tipc_topsrv_create_listener(struct tipc_topsrv *srv)
 
        saddr.family                    = AF_TIPC;
        saddr.addrtype                  = TIPC_ADDR_NAMESEQ;
-       saddr.addr.nameseq.type         = TIPC_TOP_SRV;
+       saddr.addr.nameseq.type         = TIPC_TOP_SRV;
        saddr.addr.nameseq.lower        = TIPC_TOP_SRV;
        saddr.addr.nameseq.upper        = TIPC_TOP_SRV;
        saddr.scope                     = TIPC_NODE_SCOPE;
 
-       rc = kernel_bind(lsock, (struct sockaddr *)&saddr, sizeof(saddr));
+       rc = tipc_sk_bind(lsock, (struct sockaddr *)&saddr, sizeof(saddr));
        if (rc < 0)
                goto err;
        rc = kernel_listen(lsock, 0);