netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: butt3rflyh4ck <butterflyhuangxx@gmail.com>
To: isdn@linux-pingi.de, arnd@arndb.de,
	"David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: There is an array-index-out-bounds bug in detach_capi_ctr in drivers/isdn/capi/kcapi.c
Date: Fri, 24 Sep 2021 16:44:21 +0800	[thread overview]
Message-ID: <CAFcO6XOvGQrRTaTkaJ0p3zR7y7nrAWD79r48=L_BbOyrK9X-vA@mail.gmail.com> (raw)

Hi, there is an array-index-out-bounds bug in detach_capi_ctr in
drivers/isdn/capi/kcapi.c and I reproduce it on 5.15.0-rc2+.

###Analyze
 we can call CMTPCONNADD ioctl and it would invoke
do_cmtp_sock_ioctl(), it would call cmtp_add_connection().
the chain of call is as follows.
ioctl(CMTPCONNADD)
   ->cmtp_sock_ioctl()
         -->do_cmtp_sock_ioctl()
            --->cmtp_add_connection()
                ---->kthread_run()
                ---->cmtp_attach_device()
the function would add a cmtp session to a controller. Let us see the code.
```
int cmtp_add_connection(struct cmtp_connadd_req *req, struct socket *sock)
{
u32 valid_flags = BIT(CMTP_LOOPBACK);
struct cmtp_session *session, *s;
int i, err;

BT_DBG("");

if (!l2cap_is_socket(sock))
return -EBADFD;

if (req->flags & ~valid_flags)
return -EINVAL;

session = kzalloc(sizeof(struct cmtp_session), GFP_KERNEL);   ///
alloc and clear struct cmtp_session 'session' by kzalloc().
if (!session)
return -ENOMEM;

[...]

session->task = kthread_run(cmtp_session, session,
"kcmtpd_ctr_%d",session->num); ///  create and run a kernel thread
invoke cmtp_session() and the args is 'session'

[...]

if (!(session->flags & BIT(CMTP_LOOPBACK))) {
err = cmtp_attach_device(session);   ///   invoke cmtp_attach_device()
to attach a session with a controller.
if (err < 0) {
/* Caller will call fput in case of failure, and so
* will cmtp_session kthread.
*/
get_file(session->sock->file);

atomic_inc(&session->terminate);
wake_up_interruptible(sk_sleep(session->sock->sk));
up_write(&cmtp_session_sem);
return err;
}
}
[...]

```
the struct cmtp_session have a member named struct capi_ctr 'ctrl'.
struct capi_ctr have a member named 'cnr', it is controller number.

the kernel thread would invoke cmtp_session(), it would call
cmtp_detach_device() to detach a session and the registration of a
controller.
cmtp_session()
     ->cmtp_detach_device()
          ->detach_capi_ctr()
let us see detach_capi_ctr() function implement.
```
int detach_capi_ctr(struct capi_ctr *ctr)
{
int err = 0;

mutex_lock(&capi_controller_lock);

ctr_down(ctr, CAPI_CTR_DETACHED);

if (capi_controller[ctr->cnr - 1] != ctr) {   /// use
cmtp_session->capi_ctr->cnr
err = -EINVAL;
goto unlock_out;
}
capi_controller[ctr->cnr - 1] = NULL;
ncontrollers--;

[...]

```
If the cmtp_add_connection() call cmtp_attach_device() not yet, the
cmtp_session->capi_ctr->cnr just is an ZERO.

The capi_controller[-1] make no sense. so should check that the
cmtp_session->capi_ctr->cnr is not an ZERO.


###Crash
root@syzkaller:/home/user# ./detach_capi_ctr
[   41.829295][    C1] random: crng init done
[   41.829769][    C1] random: 7 urandom warning(s) missed due to ratelimiting
[   43.904027][ T2627] Bluetooth: hci0: command 0x0409 tx timeout
[   45.984105][ T2911] Bluetooth: hci0: command 0x041b tx timeout
[   46.863815][ T6447] Bluetooth: Found 0 CAPI controller(s) on device
10:aa:aa:aa:aa:aa
[   46.864775][ T6479]
================================================================================
[   46.866069][ T6479] UBSAN: array-index-out-of-bounds in
drivers/isdn/capi/kcapi.c:483:21
[   46.867196][ T6479] index -1 is out of range for type 'capi_ctr *[32]'
[   46.867982][ T6479] CPU: 1 PID: 6479 Comm: kcmtpd_ctr_0 Not tainted
5.15.0-rc2+ #8
[   46.869002][ T6479] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.14.0-2 04/01/2014
[   46.870107][ T6479] Call Trace:
[   46.870473][ T6479]  dump_stack_lvl+0x57/0x7d
[   46.870974][ T6479]  ubsan_epilogue+0x5/0x40
[   46.871458][ T6479]  __ubsan_handle_out_of_bounds.cold+0x43/0x48
[   46.872135][ T6479]  detach_capi_ctr+0x64/0xc0
[   46.872639][ T6479]  cmtp_session+0x5c8/0x5d0
[   46.873131][ T6479]  ? __init_waitqueue_head+0x60/0x60
[   46.873712][ T6479]  ? cmtp_add_msgpart+0x120/0x120
[   46.874256][ T6479]  kthread+0x147/0x170
[   46.874709][ T6479]  ? set_kthread_struct+0x40/0x40
[   46.875248][ T6479]  ret_from_fork+0x1f/0x30
[   46.875773][ T6479]
================================================================================
[   46.876799][ T6479] Kernel panic - not syncing: panic_on_warn set ...
[   46.877541][ T6479] CPU: 1 PID: 6479 Comm: kcmtpd_ctr_0 Not tainted
5.15.0-rc2+ #8
[   46.878384][ T6479] Hardware name: QEMU Standard PC (i440FX + PIIX,
1996), BIOS 1.14.0-2 04/01/2014
[   46.879377][ T6479] Call Trace:
[   46.879742][ T6479]  dump_stack_lvl+0x57/0x7d
[   46.880251][ T6479]  panic+0x139/0x302
[   46.880699][ T6479]  ubsan_epilogue+0x3f/0x40
[   46.881199][ T6479]  __ubsan_handle_out_of_bounds.cold+0x43/0x48
[   46.881890][ T6479]  detach_capi_ctr+0x64/0xc0
[   46.882389][ T6479]  cmtp_session+0x5c8/0x5d0
[   46.882881][ T6479]  ? __init_waitqueue_head+0x60/0x60
[   46.883448][ T6479]  ? cmtp_add_msgpart+0x120/0x120
[   46.883989][ T6479]  kthread+0x147/0x170
[   46.884435][ T6479]  ? set_kthread_struct+0x40/0x40
[   46.884989][ T6479]  ret_from_fork+0x1f/0x30
[   46.885561][ T6479] Kernel Offset: disabled
[   46.886043][ T6479] Rebooting in 86400 seconds..

###Patch
Should check the cmtp_session->capi_ctr->cnr is not an ZERO.

```
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index cb0afe897162..38502a955f13 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -480,7 +480,7 @@ int detach_capi_ctr(struct capi_ctr *ctr)

        ctr_down(ctr, CAPI_CTR_DETACHED);

-       if (capi_controller[ctr->cnr - 1] != ctr) {
+       if (!ctr->cnr || capi_controller[ctr->cnr - 1] != ctr) {
                err = -EINVAL;
                goto unlock_out;
        }
```


Regards,
  butt3rflyh4ck.

-- 
Active Defense Lab of Venustech

             reply	other threads:[~2021-09-24  8:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-24  8:44 butt3rflyh4ck [this message]
2021-09-24  9:19 ` Arnd Bergmann
2021-09-24 10:02   ` butt3rflyh4ck
2021-09-28  8:35     ` butt3rflyh4ck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFcO6XOvGQrRTaTkaJ0p3zR7y7nrAWD79r48=L_BbOyrK9X-vA@mail.gmail.com' \
    --to=butterflyhuangxx@gmail.com \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=isdn@linux-pingi.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: There is an array-index-out-bounds bug in detach_capi_ctr in drivers/isdn/capi/kcapi.c' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).