net/rds: An rds_sock is added too early to the hash table
authorKa-Cheong Poon <ka-cheong.poon@oracle.com>
Wed, 11 Sep 2019 09:58:05 +0000 (02:58 -0700)
committerDavid S. Miller <davem@davemloft.net>
Wed, 11 Sep 2019 14:05:40 +0000 (15:05 +0100)
In rds_bind(), an rds_sock is added to the RDS bind hash table before
rs_transport is set.  This means that the socket can be found by the
receive code path when rs_transport is NULL.  And the receive code
path de-references rs_transport for congestion update check.  This can
cause a panic.  An rds_sock should not be added to the bind hash table
before all the needed fields are set.

Reported-by: syzbot+4b4f8163c2e246df3c4c@syzkaller.appspotmail.com
Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/rds/bind.c

index 0f4398e7f2a7add7c20b6fdd333c40af4e719c92..05464fd7c17afcc708eec3e4305ca85d039048d3 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2006, 2018 Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2019 Oracle and/or its affiliates. All rights reserved.
  *
  * This software is available to you under a choice of one of two
  * licenses.  You may choose to be licensed under the terms of the GNU
@@ -239,34 +239,30 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
                goto out;
        }
 
-       sock_set_flag(sk, SOCK_RCU_FREE);
-       ret = rds_add_bound(rs, binding_addr, &port, scope_id);
-       if (ret)
-               goto out;
-
-       if (rs->rs_transport) { /* previously bound */
+       /* The transport can be set using SO_RDS_TRANSPORT option before the
+        * socket is bound.
+        */
+       if (rs->rs_transport) {
                trans = rs->rs_transport;
                if (trans->laddr_check(sock_net(sock->sk),
                                       binding_addr, scope_id) != 0) {
                        ret = -ENOPROTOOPT;
-                       rds_remove_bound(rs);
-               } else {
-                       ret = 0;
+                       goto out;
                }
-               goto out;
-       }
-       trans = rds_trans_get_preferred(sock_net(sock->sk), binding_addr,
-                                       scope_id);
-       if (!trans) {
-               ret = -EADDRNOTAVAIL;
-               rds_remove_bound(rs);
-               pr_info_ratelimited("RDS: %s could not find a transport for %pI6c, load rds_tcp or rds_rdma?\n",
-                                   __func__, binding_addr);
-               goto out;
+       } else {
+               trans = rds_trans_get_preferred(sock_net(sock->sk),
+                                               binding_addr, scope_id);
+               if (!trans) {
+                       ret = -EADDRNOTAVAIL;
+                       pr_info_ratelimited("RDS: %s could not find a transport for %pI6c, load rds_tcp or rds_rdma?\n",
+                                           __func__, binding_addr);
+                       goto out;
+               }
+               rs->rs_transport = trans;
        }
 
-       rs->rs_transport = trans;
-       ret = 0;
+       sock_set_flag(sk, SOCK_RCU_FREE);
+       ret = rds_add_bound(rs, binding_addr, &port, scope_id);
 
 out:
        release_sock(sk);