On 10/10/20 10:56 PM, 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 types which were really in use
> ([0,1]), and block all the rest ([2,63]).
>
> This is dangerous, 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., practically all users.
>
Sorry, Jon. I could not understand how such scenarios described above
happened. Can you please take an example to demonstrate a regular user
can bind to the same service type as TIPC_TOP_SRV or TIPC_CFG_SRV under
the current code logic?
Thanks,
Ying
> 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 easily be filtered out, since it is the very first
> binding performed when the system is starting.
>
> We can hence block TIPC_CFG_SRV completely, and only allow TIPC_TOP_SRV
> to be bound once, and the correct behavior is achieved. This is what we
> do in this commit.
>
> 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>
> ---
> net/tipc/socket.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/tipc/socket.c b/net/tipc/socket.c
> index e795a8a2955b..67875a5761d0 100644
> --- a/net/tipc/socket.c
> +++ b/net/tipc/socket.c
> @@ -665,6 +665,7 @@ static int tipc_bind(struct socket *sock, struct sockaddr
> *uaddr,
> struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr;
> struct tipc_sock *tsk = tipc_sk(sk);
> int res = -EINVAL;
> + u32 stype, dnode;
>
> lock_sock(sk);
> if (unlikely(!uaddr_len)) {
> @@ -691,9 +692,10 @@ 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)) {
> + stype = addr->addr.nameseq.type;
> + if (stype < TIPC_RESERVED_TYPES &&
> + (stype != TIPC_TOP_SRV ||
> + tipc_nametbl_translate(sock_net(sk), stype, stype, &dnode))) {
> res = -EACCES;
> goto exit;
> }
>
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
