[PATCH] tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux

[PATCH] tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux

[Yi Yang]

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
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>
---
 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 c7a787f10a9c..b32d91ccddda 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -3042,12 +3042,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 = true;
 	mutex_lock(&gsm->mutex);
 
+	dlci = gsm->dlci[0];
 	if (dlci) {
 		if (disc && dlci->state != DLCI_CLOSED) {
 			gsm_dlci_begin_close(dlci);
-- 
2.17.1

RE: [PATCH] tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux

[Starke, Daniel]

Looks logical and plausible to me.
Tests on our side did not show any side effect.

Reviewed-by: Daniel Starke <daniel.starke@siemens.com>

Best regards,
Daniel Starke

Re: [PATCH] tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux

[gregkh]

On Fri, Aug 18, 2023 at 05:19:10AM +0000, Starke, Daniel wrote:
> Looks logical and plausible to me.
> Tests on our side did not show any side effect.
> 
> Reviewed-by: Daniel Starke <daniel.starke@siemens.com>

Thanks for the review, but this is alreay in the tree.

greg k-h