Fix CVE-2023-6546
Daniel Starke (1): tty: n_gsm: fix restart handling via CLD command
Yi Yang (1): tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux
drivers/tty/n_gsm.c | 71 ++++++++++++++------------------------------- 1 file changed, 22 insertions(+), 49 deletions(-)
From: Daniel Starke daniel.starke@siemens.com
mainline inclusion from mainline-v5.18-rc5 commit aa371e96f05dcb36a88298f5cb70aa7234d5e8b8 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I8QFUO CVE: CVE-2023-6546
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
n_gsm is based on the 3GPP 07.010 and its newer version is the 3GPP 27.010. See https://portal.3gpp.org/desktopmodules/Specifications/SpecificationDetails.a... The changes from 07.010 to 27.010 are non-functional. Therefore, I refer to the newer 27.010 here. Chapter 5.8.2 states that both sides will revert to the non-multiplexed mode via a close-down message (CLD). The usual program flow is as following: - start multiplex mode by sending AT+CMUX to the mobile - establish the control channel (DLCI 0) - establish user channels (DLCI >0) - terminate user channels - send close-down message (CLD) - revert to AT protocol (i.e. leave multiplexed mode)
The AT protocol is out of scope of the n_gsm driver. However, gsm_disconnect() sends CLD if gsm_config() detects that the requested parameters require the mux protocol to restart. The next immediate action is to start the mux protocol by opening DLCI 0 again. Any responder side which handles CLD commands correctly forces us to fail at this point because AT+CMUX needs to be sent to the mobile to start the mux again. Therefore, remove the CLD command in this phase and keep both sides in multiplexed mode. Remove the gsm_disconnect() function as it become unnecessary and merge the remaining parts into gsm_cleanup_mux() to handle the termination order and locking correctly.
Fixes: 71e077915396 ("tty: n_gsm: do not send/receive in ldisc close path") Cc: stable@vger.kernel.org Signed-off-by: Daniel Starke daniel.starke@siemens.com Link: https://lore.kernel.org/r/20220414094225.4527-2-daniel.starke@siemens.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
conflicts: drivers/tty/n_gsm.c
Signed-off-by: Yi Yang yiyang13@huawei.com --- drivers/tty/n_gsm.c | 68 +++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 48 deletions(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 3f08ad7e629a..ccb511dc8898 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2024,49 +2024,35 @@ static void gsm_error(struct gsm_mux *gsm, gsm->io_error++; }
-static int gsm_disconnect(struct gsm_mux *gsm) -{ - struct gsm_dlci *dlci = gsm->dlci[0]; - struct gsm_control *gc; - - if (!dlci) - return 0; - - /* In theory disconnecting DLCI 0 is sufficient but for some - modems this is apparently not the case. */ - gc = gsm_control_send(gsm, CMD_CLD, NULL, 0); - if (gc) - gsm_control_wait(gsm, gc); - - del_timer_sync(&gsm->t2_timer); - /* Now we are sure T2 has stopped */ - - gsm_dlci_begin_close(dlci); - wait_event_interruptible(gsm->event, - dlci->state == DLCI_CLOSED); - - if (signal_pending(current)) - return -EINTR; - - return 0; -} - /** * gsm_cleanup_mux - generic GSM protocol cleanup * @gsm: our mux + * @disc: disconnect link? * * Clean up the bits of the mux which are the same for all framing * protocols. Remove the mux from the mux table, stop all the timers * and then shut down each device hanging up the channels as we go. */
-static void gsm_cleanup_mux(struct gsm_mux *gsm) +static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) { int i; struct gsm_dlci *dlci = gsm->dlci[0]; struct gsm_msg *txq, *ntxq;
gsm->dead = 1; + mutex_lock(&gsm->mutex); + + if (dlci) { + if (disc && dlci->state != DLCI_CLOSED) { + gsm_dlci_begin_close(dlci); + wait_event(gsm->event, dlci->state == DLCI_CLOSED); + } + dlci->dead = true; + } + + /* Finish outstanding timers, making sure they are done */ + del_timer_sync(&gsm->t2_timer);
spin_lock(&gsm_mux_lock); for (i = 0; i < MAX_MUX; i++) { @@ -2080,13 +2066,7 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm) if (i == MAX_MUX) return;
- del_timer_sync(&gsm->t2_timer); - /* Now we are sure T2 has stopped */ - if (dlci) - dlci->dead = 1; - /* Free up any link layer users */ - mutex_lock(&gsm->mutex); for (i = 0; i < NUM_DLCI; i++) if (gsm->dlci[i]) gsm_dlci_release(gsm->dlci[i]); @@ -2285,7 +2265,7 @@ static void gsmld_detach_gsm(struct tty_struct *tty, struct gsm_mux *gsm) WARN_ON(tty != gsm->tty); for (i = 1; i < NUM_DLCI; i++) tty_unregister_device(gsm_tty_driver, base + i); - gsm_cleanup_mux(gsm); + gsm_cleanup_mux(gsm, false); tty_kref_put(gsm->tty); gsm->tty = NULL; } @@ -2391,7 +2371,7 @@ static int gsmld_open(struct tty_struct *tty)
ret = gsmld_attach_gsm(tty, gsm); if (ret != 0) { - gsm_cleanup_mux(gsm); + gsm_cleanup_mux(gsm, false); mux_put(gsm); } return ret; @@ -2540,19 +2520,11 @@ static int gsmld_config(struct tty_struct *tty, struct gsm_mux *gsm,
/* * Close down what is needed, restart and initiate the new - * configuration + * configuration. On the first time there is no DLCI[0] + * and closing or cleaning up is not necessary. */ - - if (need_close || need_restart) { - int ret; - - ret = gsm_disconnect(gsm); - - if (ret) - return ret; - } - if (need_restart) - gsm_cleanup_mux(gsm); + if (need_close || need_restart) + gsm_cleanup_mux(gsm, true);
gsm->initiator = c->initiator; gsm->mru = c->mru;
mainline inclusion from mainline-v6.5-rc7 commit 3c4f8333b582487a2d1e02171f1465531cde53e3 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/I8QFUO CVE: CVE-2023-6546
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
In commit 9b9c8195f3f0 ("tty: n_gsm: fix UAF in gsm_cleanup_mux"), the UAF problem is not completely fixed. There is a race condition in gsm_cleanup_mux(), which caused this UAF.
The UAF problem is triggered by the following race: task[5046] task[5054] ----------------------- ----------------------- gsm_cleanup_mux(); dlci = gsm->dlci[0]; mutex_lock(&gsm->mutex); gsm_cleanup_mux(); dlci = gsm->dlci[0]; //Didn't take the lock gsm_dlci_release(gsm->dlci[i]); gsm->dlci[i] = NULL; mutex_unlock(&gsm->mutex); mutex_lock(&gsm->mutex); dlci->dead = true; //UAF
Fix it by assigning values after mutex_lock().
Link: https://syzkaller.appspot.com/text?tag=CrashReport&x=176188b5a80000 Cc: stable stable@kernel.org Fixes: 9b9c8195f3f0 ("tty: n_gsm: fix UAF in gsm_cleanup_mux") Fixes: aa371e96f05d ("tty: n_gsm: fix restart handling via CLD command") Signed-off-by: Yi Yang yiyang13@huawei.com Co-developed-by: Qiumiao Zhang zhangqiumiao1@huawei.com Signed-off-by: Qiumiao Zhang zhangqiumiao1@huawei.com Link: https://lore.kernel.org/r/20230811031121.153237-1-yiyang13@huawei.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
conflicts: drivers/tty/n_gsm.c
Signed-off-by: Yi Yang yiyang13@huawei.com --- drivers/tty/n_gsm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index ccb511dc8898..05d9064b4ee7 100644 --- a/drivers/tty/n_gsm.c +++ b/drivers/tty/n_gsm.c @@ -2037,12 +2037,13 @@ static void gsm_error(struct gsm_mux *gsm, static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc) { int i; - struct gsm_dlci *dlci = gsm->dlci[0]; + struct gsm_dlci *dlci; struct gsm_msg *txq, *ntxq;
gsm->dead = 1; mutex_lock(&gsm->mutex);
+ dlci = gsm->dlci[0]; if (dlci) { if (disc && dlci->state != DLCI_CLOSED) { gsm_dlci_begin_close(dlci);
反馈: 您发送到kernel@openeuler.org的补丁/补丁集,已成功转换为PR! PR链接地址: https://gitee.com/openeuler/kernel/pulls/3607 邮件列表地址:https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/B...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/3607 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/B...