Bluetooth: RFCOMM: Fix session UAF in set_termios

rfcomm_tty_set_termios() tests dlc->session without rfcomm_mutex and
later passes the pointer to rfcomm_send_rpn(). The latter dereferences
both session->initiator and session->sock. Meanwhile, krfcommd can
unlink the DLC and free the session while holding rfcomm_mutex.

The race can proceed as follows:

  TTY ioctl task                 krfcommd
  --------------                 --------
  load dlc->session
  enter rfcomm_send_rpn()
                                 lock rfcomm_mutex
                                 clear dlc->session
                                 free session
                                 unlock rfcomm_mutex
  read session->initiator

KASAN reported:

  BUG: KASAN: slab-use-after-free in rfcomm_send_rpn+0x297/0x2a0
  Read of size 4 at addr ffff88810012a850 by task poc/92

  Call Trace:
   rfcomm_send_rpn+0x297/0x2a0
   rfcomm_tty_set_termios+0x50d/0x850
   tty_set_termios+0x596/0x950
   set_termios+0x46a/0x6e0
   tty_mode_ioctl+0x152/0xbd0
   tty_ioctl+0x915/0x1240
   __x64_sys_ioctl+0x134/0x1c0

  Allocated by task 92:
   rfcomm_session_add+0x9e/0x2e0
   rfcomm_dlc_open+0x8b1/0xe00
   rfcomm_dev_activate+0x85/0x1a0
   rfcomm_tty_open+0x90/0x280

  Freed by task 68:
   kfree+0x131/0x3c0
   rfcomm_session_del+0x119/0x180
   rfcomm_run+0x737/0x4710

Add rfcomm_dlc_send_rpn(), which holds rfcomm_mutex while it verifies
that the DLC is still attached and sends the RPN frame. Have the TTY
path use the helper and drop its unlocked session check. This keeps the
session valid through both the frame construction and socket send.

Fixes: 3a5e903c09ae ("[Bluetooth]: Implement RFCOMM remote port negotiation")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index feb6b3ae5e6946770cbb6357085ad2c7656f6f75..102c278e35844c5ae31706f99c540297a6b29f61 100644 (file)
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -226,6 +226,9 @@ int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
                        u8 bit_rate, u8 data_bits, u8 stop_bits,
                        u8 parity, u8 flow_ctrl_settings,
                        u8 xon_char, u8 xoff_char, u16 param_mask);
+int rfcomm_dlc_send_rpn(struct rfcomm_dlc *d, u8 bit_rate, u8 data_bits,
+                       u8 stop_bits, u8 parity, u8 flow_ctrl_settings,
+                       u8 xon_char, u8 xoff_char, u16 param_mask);
 
 /* ---- RFCOMM DLCs (channels) ---- */
 struct rfcomm_dlc *rfcomm_dlc_alloc(gfp_t prio);

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index ebeae17b71d121559b509795f81461ca9f95eb3e..75f7512dec54285e39f26488ac7be966e8b0df87 100644 (file)
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1028,6 +1028,23 @@ int rfcomm_send_rpn(struct rfcomm_session *s, int cr, u8 dlci,
        return rfcomm_send_frame(s, buf, ptr - buf);
 }
 
+int rfcomm_dlc_send_rpn(struct rfcomm_dlc *d, u8 bit_rate, u8 data_bits,
+                       u8 stop_bits, u8 parity, u8 flow_ctrl_settings,
+                       u8 xon_char, u8 xoff_char, u16 param_mask)
+{
+       int err = -ENOTCONN;
+
+       rfcomm_lock();
+       if (d->session)
+               err = rfcomm_send_rpn(d->session, 1, d->dlci, bit_rate,
+                                     data_bits, stop_bits, parity,
+                                     flow_ctrl_settings, xon_char, xoff_char,
+                                     param_mask);
+       rfcomm_unlock();
+
+       return err;
+}
+
 static int rfcomm_send_rls(struct rfcomm_session *s, int cr, u8 dlci, u8 status)
 {
        struct rfcomm_hdr *hdr;

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 4b9a699ec59b5a35f75b8a8c27e714786d3aa40b..b2c1060394e601de1d2922553f641dfe645cd550 100644 (file)
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -858,7 +858,7 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty,
 
        BT_DBG("tty %p termios %p", tty, old);
 
-       if (!dev || !dev->dlc || !dev->dlc->session)
+       if (!dev || !dev->dlc)
                return;
 
        /* Handle turning off CRTSCTS */
@@ -979,9 +979,8 @@ static void rfcomm_tty_set_termios(struct tty_struct *tty,
        }
 
        if (changes)
-               rfcomm_send_rpn(dev->dlc->session, 1, dev->dlc->dlci, baud,
-                               data_bits, stop_bits, parity,
-                               RFCOMM_RPN_FLOW_NONE, x_on, x_off, changes);
+               rfcomm_dlc_send_rpn(dev->dlc, baud, data_bits, stop_bits, parity,
+                                   RFCOMM_RPN_FLOW_NONE, x_on, x_off, changes);
 }
 
 static void rfcomm_tty_throttle(struct tty_struct *tty)