From 4482177818ca24cb8c6aa35007573be3b69e0316 Mon Sep 17 00:00:00 2001 From: Lin Ma Date: Wed, 23 Jun 2021 09:09:02 +0800 Subject: [PATCH] Bluetooth: fix the UAF without introducing the lock bug This patch revert the previous one (e305509e678b3a4af2b3cfd410f409f7cdaabb52) which will trigger a lock bug when enabling CONFIG_DEBUG_ATOMIC_SLEEP. At the same time, to make sure that the hdev object will not be released when hci_sock_bound_ioctl() and hci_sock_sendmsg() functions still hold its reference. This patch utilizes refcount methods in these two functions. Signed-off-by: Lin Ma --- net/bluetooth/hci_conn.c | 6 ++++ net/bluetooth/hci_sock.c | 76 ++++++++++++++++++++++++++++------------ 2 files changed, 59 insertions(+), 23 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 88ec08978ff4..2787da8fe14a 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1711,6 +1711,9 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&req, arg, sizeof(req))) return -EFAULT; + if (!test_bit(HCI_UP, &hdev->flags)) + return -ENETDOWN; + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, req.type, &req.bdaddr); if (conn) { @@ -1737,6 +1740,9 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&req, arg, sizeof(req))) return -EFAULT; + if (!test_bit(HCI_UP, &hdev->flags)) + return -ENETDOWN; + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr); if (conn) diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index eed0dd066e12..a7a9e5c55ffd 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -762,7 +762,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event) /* Detach sockets from device */ read_lock(&hci_sk_list.lock); sk_for_each(sk, &hci_sk_list.head) { - lock_sock(sk); + bh_lock_sock_nested(sk); if (hci_pi(sk)->hdev == hdev) { hci_pi(sk)->hdev = NULL; sk->sk_err = EPIPE; @@ -771,7 +771,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event) hci_dev_put(hdev); } - release_sock(sk); + bh_unlock_sock(sk); } read_unlock(&hci_sk_list.lock); } @@ -900,6 +900,9 @@ static int hci_sock_blacklist_add(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&bdaddr, arg, sizeof(bdaddr))) return -EFAULT; + if (!test_bit(HCI_UP, &hdev->flags)) + return -ENETDOWN; + hci_dev_lock(hdev); err = hci_bdaddr_list_add(&hdev->blacklist, &bdaddr, BDADDR_BREDR); @@ -917,6 +920,9 @@ static int hci_sock_blacklist_del(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&bdaddr, arg, sizeof(bdaddr))) return -EFAULT; + if (!test_bit(HCI_UP, &hdev->flags)) + return -ENETDOWN; + hci_dev_lock(hdev); err = hci_bdaddr_list_del(&hdev->blacklist, &bdaddr, BDADDR_BREDR); @@ -931,43 +937,64 @@ static int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd, unsigned long arg) { struct hci_dev *hdev = hci_pi(sk)->hdev; + int err; if (!hdev) return -EBADFD; - if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) - return -EBUSY; + hdev = hci_dev_hold(hdev); - if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) - return -EOPNOTSUPP; + if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { + err = -EBUSY; + goto done; + } - if (hdev->dev_type != HCI_PRIMARY) - return -EOPNOTSUPP; + if (hci_dev_test_flag(hdev, HCI_UNCONFIGURED)) { + err = -EOPNOTSUPP; + goto done; + } + + if (hdev->dev_type != HCI_PRIMARY) { + err = -EOPNOTSUPP; + goto done; + } switch (cmd) { case HCISETRAW: if (!capable(CAP_NET_ADMIN)) - return -EPERM; - return -EOPNOTSUPP; + err = -EPERM; + else + err = -EOPNOTSUPP; + break; case HCIGETCONNINFO: - return hci_get_conn_info(hdev, (void __user *)arg); + err = hci_get_conn_info(hdev, (void __user *)arg); + break; case HCIGETAUTHINFO: - return hci_get_auth_info(hdev, (void __user *)arg); + err = hci_get_auth_info(hdev, (void __user *)arg); + break; case HCIBLOCKADDR: if (!capable(CAP_NET_ADMIN)) - return -EPERM; - return hci_sock_blacklist_add(hdev, (void __user *)arg); + err = -EPERM; + else + err = hci_sock_blacklist_add(hdev, (void __user *)arg); + break; case HCIUNBLOCKADDR: if (!capable(CAP_NET_ADMIN)) - return -EPERM; - return hci_sock_blacklist_del(hdev, (void __user *)arg); + err = -EPERM; + else + err = hci_sock_blacklist_del(hdev, (void __user *)arg); + break; + default: + err = -ENOIOCTLCMD; } - return -ENOIOCTLCMD; +done: + hci_dev_put(hdev); + return err; } static int hci_sock_ioctl(struct socket *sock, unsigned int cmd, @@ -1745,14 +1772,11 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, hdev = hci_pi(sk)->hdev; if (!hdev) { - err = -EBADFD; - goto done; + release_sock(sk); + return -EBADFD; } - if (!test_bit(HCI_UP, &hdev->flags)) { - err = -ENETDOWN; - goto done; - } + hdev = hci_dev_hold(hdev); skb = bt_skb_send_alloc(sk, len, msg->msg_flags & MSG_DONTWAIT, &err); if (!skb) @@ -1763,6 +1787,11 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, goto drop; } + if (!test_bit(HCI_UP, &hdev->flags)) { + err = -ENETDOWN; + goto drop; + } + hci_skb_pkt_type(skb) = skb->data[0]; skb_pull(skb, 1); @@ -1832,6 +1861,7 @@ static int hci_sock_sendmsg(struct socket *sock, struct msghdr *msg, err = len; done: + hci_dev_put(hdev); release_sock(sk); return err; -- 2.32.0