git: 7a70f17ac4bd - main - killpg(): more carefully avoid LoR

[Konstantin Belousov]

The branch main has been updated by kib:

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

commit 7a70f17ac4bd64dc1a5020f963ba4380cf37b7e5
Author:     Konstantin Belousov <k...@freebsd.org>
AuthorDate: 2023-07-07 17:19:33 +0000
Commit:     Konstantin Belousov <k...@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);
 }