mISDN: fix use-after-free bugs in l1oip timer handlers

[ Upstream commit 2568a7e0832ee30b0a351016d03062ab4e0e0a3f ]

The l1oip_cleanup() traverses the l1oip_ilist and calls
release_card() to cleanup module and stack. However,
release_card() calls del_timer() to delete the timers
such as keep_tl and timeout_tl. If the timer handler is
running, the del_timer() will not stop it and result in
UAF bugs. One of the processes is shown below:

    (cleanup routine)          |        (timer handler)
release_card()                 | l1oip_timeout()
 ...                           |
 del_timer()                   | ...
 ...                           |
 kfree(hc) //FREE              |
                               | hc->timeout_on = 0 //USE

Fix by calling del_timer_sync() in release_card(), which
makes sure the timer handlers have finished before the
resources, such as l1oip and so on, have been deallocated.

What's more, the hc->workq and hc->socket_thread can kick
those timers right back in. We add a bool flag to show
if card is released. Then, check this flag in hc->workq
and hc->socket_thread.

Fixes: 3712b42d4b1b ("Add layer1 over IP support")
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/isdn/mISDN/l1oip.h b/drivers/isdn/mISDN/l1oip.h
index 7ea10db20e3a6567d91b4d6d8b63daa99eadaa98..48133d022812071543fb4e0fee48af6968619f2d 100644 (file)
--- a/drivers/isdn/mISDN/l1oip.h
+++ b/drivers/isdn/mISDN/l1oip.h
@@ -59,6 +59,7 @@ struct l1oip {
        int                     bundle;         /* bundle channels in one frm */
        int                     codec;          /* codec to use for transmis. */
        int                     limit;          /* limit number of bchannels */
+       bool                    shutdown;       /* if card is released */
 
        /* timer */
        struct timer_list       keep_tl;

diff --git a/drivers/isdn/mISDN/l1oip_core.c b/drivers/isdn/mISDN/l1oip_core.c
index 2c40412466e6fb39fb64c155b6337e382c1cedfa..a77195e378b7ba425c3838e8e98db03c27acfef0 100644 (file)
--- a/drivers/isdn/mISDN/l1oip_core.c
+++ b/drivers/isdn/mISDN/l1oip_core.c
@@ -275,7 +275,7 @@ l1oip_socket_send(struct l1oip *hc, u8 localcodec, u8 channel, u32 chanmask,
        p = frame;
 
        /* restart timer */
-       if (time_before(hc->keep_tl.expires, jiffies + 5 * HZ))
+       if (time_before(hc->keep_tl.expires, jiffies + 5 * HZ) && !hc->shutdown)
                mod_timer(&hc->keep_tl, jiffies + L1OIP_KEEPALIVE * HZ);
        else
                hc->keep_tl.expires = jiffies + L1OIP_KEEPALIVE * HZ;
@@ -601,7 +601,9 @@ multiframe:
                goto multiframe;
 
        /* restart timer */
-       if (time_before(hc->timeout_tl.expires, jiffies + 5 * HZ) || !hc->timeout_on) {
+       if ((time_before(hc->timeout_tl.expires, jiffies + 5 * HZ) ||
+            !hc->timeout_on) &&
+           !hc->shutdown) {
                hc->timeout_on = 1;
                mod_timer(&hc->timeout_tl, jiffies + L1OIP_TIMEOUT * HZ);
        } else /* only adjust timer */
@@ -1232,11 +1234,10 @@ release_card(struct l1oip *hc)
 {
        int     ch;
 
-       if (timer_pending(&hc->keep_tl))
-               del_timer(&hc->keep_tl);
+       hc->shutdown = true;
 
-       if (timer_pending(&hc->timeout_tl))
-               del_timer(&hc->timeout_tl);
+       del_timer_sync(&hc->keep_tl);
+       del_timer_sync(&hc->timeout_tl);
 
        cancel_work_sync(&hc->workq);