Skip to content

Commit 09e50aa

Browse files
ciq-sahlbergPlaidCat
authored andcommitted
tty: n_gsm: fix the UAF caused by race condition in gsm_cleanup_mux
jira SECO-49 cve CVE-2023-6546 commit 3c4f833 upstream-diff we are missing several refactors of this file from upstream and while we do not have Fixes commits in this tree, the same bug that this addresses "grabbing the dlci pointer before taking the mutex" still exists before those two commits. In this case it is not that the pointer can be NULLed while the second thread waits for the mutex but that the pointer could be free()d in the call to gsm_dlci_release() and no longer be valid. This could cause memory corruption as we will write to the area pointed to. In commit 9b9c819 ("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 <[email protected]> Fixes: 9b9c819 ("tty: n_gsm: fix UAF in gsm_cleanup_mux") Fixes: aa371e9 ("tty: n_gsm: fix restart handling via CLD command") Signed-off-by: Yi Yang <[email protected]> Co-developed-by: Qiumiao Zhang <[email protected]> Signed-off-by: Qiumiao Zhang <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Ronnie Sahlberg <[email protected]>
1 parent 902f8e4 commit 09e50aa

File tree

1 file changed

+4
-3
lines changed

1 file changed

+4
-3
lines changed

drivers/tty/n_gsm.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2061,7 +2061,7 @@ static int gsm_disconnect(struct gsm_mux *gsm)
20612061
static void gsm_cleanup_mux(struct gsm_mux *gsm)
20622062
{
20632063
int i;
2064-
struct gsm_dlci *dlci = gsm->dlci[0];
2064+
struct gsm_dlci *dlci;
20652065
struct gsm_msg *txq, *ntxq;
20662066

20672067
gsm->dead = 1;
@@ -2079,12 +2079,13 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm)
20792079
return;
20802080

20812081
del_timer_sync(&gsm->t2_timer);
2082+
/* Free up any link layer users */
2083+
mutex_lock(&gsm->mutex);
2084+
dlci = gsm->dlci[0];
20822085
/* Now we are sure T2 has stopped */
20832086
if (dlci)
20842087
dlci->dead = 1;
20852088

2086-
/* Free up any link layer users */
2087-
mutex_lock(&gsm->mutex);
20882089
for (i = 0; i < NUM_DLCI; i++)
20892090
if (gsm->dlci[i])
20902091
gsm_dlci_release(gsm->dlci[i]);

0 commit comments

Comments
 (0)