On 10/27/20 7:07 AM, jma...@redhat.com wrote:
> From: Jon Maloy <jma...@redhat.com>
>
> 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 neigboring 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.
>
> Signed-off-by: Jon Maloy <jma...@redhat.com>
Acked-by: Ying Xue <ying....@windriver.com>
> ---
> net/tipc/socket.c | 24 +++++++++++++++---------
> net/tipc/socket.h | 2 +-
> net/tipc/topsrv.c | 4 ++--
> 3 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index e795a8a2955b..222fd53da2d0 100644
> --- 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,19 @@ static int tipc_bind(struct socket *sock, struct
> sockaddr *uaddr,
> 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)
> + 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 b11575afc66f..02cdf166807d 100644
> --- 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 5f6f86051c83..cec029349662 100644
> --- 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);
>
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
