tipc: add an extra conn_get in tipc_conn_alloc
authorXin Long <lucien.xin@gmail.com>
Fri, 18 Nov 2022 21:45:01 +0000 (16:45 -0500)
committerJakub Kicinski <kuba@kernel.org>
Tue, 22 Nov 2022 04:45:24 +0000 (20:45 -0800)
One extra conn_get() is needed in tipc_conn_alloc(), as after
tipc_conn_alloc() is called, tipc_conn_close() may free this
con before deferencing it in tipc_topsrv_accept():

   tipc_conn_alloc();
   newsk = newsock->sk;
                                 <---- tipc_conn_close();
   write_lock_bh(&sk->sk_callback_lock);
   newsk->sk_data_ready = tipc_conn_data_ready;

Then an uaf issue can be triggered:

  BUG: KASAN: use-after-free in tipc_topsrv_accept+0x1e7/0x370 [tipc]
  Call Trace:
   <TASK>
   dump_stack_lvl+0x33/0x46
   print_report+0x178/0x4b0
   kasan_report+0x8c/0x100
   kasan_check_range+0x179/0x1e0
   tipc_topsrv_accept+0x1e7/0x370 [tipc]
   process_one_work+0x6a3/0x1030
   worker_thread+0x8a/0xdf0

This patch fixes it by holding it in tipc_conn_alloc(), then after
all accessing in tipc_topsrv_accept() releasing it. Note when does
this in tipc_topsrv_kern_subscr(), as tipc_conn_rcv_sub() returns
0 or -1 only, we don't need to check for "> 0".

Fixes: c5fa7b3cf3cb ("tipc: introduce new TIPC server infrastructure")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/tipc/topsrv.c

index b0f9aa521670abd6791009a817696f1a4a00bea9..e3b427a703980dc7e1967939e0f8ebd51a9f9648 100644 (file)
@@ -206,6 +206,7 @@ static struct tipc_conn *tipc_conn_alloc(struct tipc_topsrv *s, struct socket *s
        set_bit(CF_CONNECTED, &con->flags);
        con->server = s;
        con->sock = sock;
+       conn_get(con);
        spin_unlock_bh(&s->idr_lock);
 
        return con;
@@ -484,6 +485,7 @@ static void tipc_topsrv_accept(struct work_struct *work)
 
                /* Wake up receive process in case of 'SYN+' message */
                newsk->sk_data_ready(newsk);
+               conn_put(con);
        }
 }
 
@@ -583,10 +585,11 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32 port, u32 type, u32 lower,
 
        *conid = con->conid;
        rc = tipc_conn_rcv_sub(tipc_topsrv(net), con, &sub);
-       if (rc >= 0)
-               return true;
+       if (rc)
+               conn_put(con);
+
        conn_put(con);
-       return false;
+       return !rc;
 }
 
 void tipc_topsrv_kern_unsubscr(struct net *net, int conid)