Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 8 Jul 2023 19:07:31 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 7a70f17ac4bd - main - killpg(): more carefully avoid LoR
Message-ID:  <202307081907.368J7VLL011285@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=7a70f17ac4bd64dc1a5020f963ba4380cf37b7e5

commit 7a70f17ac4bd64dc1a5020f963ba4380cf37b7e5
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-07-07 17:19:33 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-07-08 19:05:25 +0000

    killpg(): more carefully avoid LoR
    
    otherwise we could end up with the livelock.  When pg_killsx trylock
    failed, ensure that we do wait for lock availability before retry.
    
    Reported and tested by: pho
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
---
 sys/kern/kern_proc.c | 15 +++++++++++++--
 sys/kern/kern_prot.c |  6 ++----
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 6d804d93eccc..b82dd07b17c1 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -587,8 +587,12 @@ enterpgrp(struct proc *p, pid_t pgid, struct pgrp *pgrp, struct session *sess)
 	    ("enterpgrp: session leader attempted setpgrp"));
 
 	old_pgrp = p->p_pgrp;
-	if (!sx_try_xlock(&old_pgrp->pg_killsx))
+	if (!sx_try_xlock(&old_pgrp->pg_killsx)) {
+		sx_xunlock(&proctree_lock);
+		sx_xlock(&old_pgrp->pg_killsx);
+		sx_xunlock(&old_pgrp->pg_killsx);
 		return (ERESTART);
+	}
 	MPASS(old_pgrp == p->p_pgrp);
 
 	if (sess != NULL) {
@@ -656,11 +660,18 @@ enterthispgrp(struct proc *p, struct pgrp *pgrp)
 	    ("%s: p %p belongs to pgrp %p", __func__, p, pgrp));
 
 	old_pgrp = p->p_pgrp;
-	if (!sx_try_xlock(&old_pgrp->pg_killsx))
+	if (!sx_try_xlock(&old_pgrp->pg_killsx)) {
+		sx_xunlock(&proctree_lock);
+		sx_xlock(&old_pgrp->pg_killsx);
+		sx_xunlock(&old_pgrp->pg_killsx);
 		return (ERESTART);
+	}
 	MPASS(old_pgrp == p->p_pgrp);
 	if (!sx_try_xlock(&pgrp->pg_killsx)) {
 		sx_xunlock(&old_pgrp->pg_killsx);
+		sx_xunlock(&proctree_lock);
+		sx_xlock(&pgrp->pg_killsx);
+		sx_xunlock(&pgrp->pg_killsx);
 		return (ERESTART);
 	}
 
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
index 1fac6c496945..46d73a471291 100644
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -347,10 +347,8 @@ again:
 		error = EPERM;
 	} else {
 		error = enterpgrp(p, p->p_pid, newpgrp, newsess);
-		if (error == ERESTART) {
-			sx_xunlock(&proctree_lock);
+		if (error == ERESTART)
 			goto again;
-		}
 		MPASS(error == 0);
 		td->td_retval[0] = p->p_pid;
 		newpgrp = NULL;
@@ -460,11 +458,11 @@ again:
 		error = enterthispgrp(targp, pgrp);
 	}
 done:
-	sx_xunlock(&proctree_lock);
 	KASSERT(error == 0 || newpgrp != NULL,
 	    ("setpgid failed and newpgrp is NULL"));
 	if (error == ERESTART)
 		goto again;
+	sx_xunlock(&proctree_lock);
 	uma_zfree(pgrp_zone, newpgrp);
 	return (error);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202307081907.368J7VLL011285>