From 596325f15455198a20231d34cbcb59089dcdf353 Mon Sep 17 00:00:00 2001 From: John Baldwin <jhb@FreeBSD.org> Date: Tue, 16 Apr 2002 17:09:22 +0000 Subject: [PATCH] - Lock proctree_lock instead of pgrpsess_lock. - Use temporary variables to hold a pointer to a pgrp while we dink with it while not holding either the associated proc lock or proctree_lock. It is in theory possible that p->p_pgrp could change out from under us. --- sys/kern/tty.c | 67 +++++++++++++++++++++++----------------------- sys/kern/tty_pty.c | 14 +++++----- 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/sys/kern/tty.c b/sys/kern/tty.c index b698977877b9..6e4ca184315d 100644 --- a/sys/kern/tty.c +++ b/sys/kern/tty.c @@ -731,6 +731,7 @@ ttioctl(tp, cmd, data, flag) { register struct proc *p; struct thread *td; + struct pgrp *pgrp; int s, error; td = curthread; /* XXX */ @@ -770,30 +771,30 @@ ttioctl(tp, cmd, data, flag) case TIOCSETP: case TIOCSLTC: #endif - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); PROC_LOCK(p); while (isbackground(p, tp) && !(p->p_flag & P_PPWAIT) && !SIGISMEMBER(p->p_sigignore, SIGTTOU) && !SIGISMEMBER(p->p_sigmask, SIGTTOU)) { - if (p->p_pgrp->pg_jobc == 0) { - PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + pgrp = p->p_pgrp; + PROC_UNLOCK(p); + if (pgrp->pg_jobc == 0) { + sx_sunlock(&proctree_lock); return (EIO); } - PROC_UNLOCK(p); - PGRP_LOCK(p->p_pgrp); - PGRPSESS_SUNLOCK(); - pgsignal(p->p_pgrp, SIGTTOU, 1); - PGRP_UNLOCK(p->p_pgrp); + PGRP_LOCK(pgrp); + sx_sunlock(&proctree_lock); + pgsignal(pgrp, SIGTTOU, 1); + PGRP_UNLOCK(pgrp); error = ttysleep(tp, &lbolt, TTOPRI | PCATCH, "ttybg1", 0); if (error) return (error); - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); PROC_LOCK(p); } PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); break; } @@ -1053,11 +1054,11 @@ ttioctl(tp, cmd, data, flag) break; case TIOCSCTTY: /* become controlling tty */ /* Session ctty vnode pointer set in vnode layer. */ - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); if (!SESS_LEADER(p) || ((p->p_session->s_ttyvp || tp->t_session) && (tp->t_session != p->p_session))) { - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (EPERM); } tp->t_session = p->p_session; @@ -1068,29 +1069,27 @@ ttioctl(tp, cmd, data, flag) PROC_LOCK(p); p->p_flag |= P_CONTROLT; PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); break; case TIOCSPGRP: { /* set pgrp of tty */ - register struct pgrp *pgrp; - - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); pgrp = pgfind(*(int *)data); if (!isctty(p, tp)) { if (pgrp != NULL) PGRP_UNLOCK(pgrp); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (ENOTTY); } if (pgrp == NULL) { - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (EPERM); } PGRP_UNLOCK(pgrp); if (pgrp->pg_session != p->p_session) { - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (EPERM); } - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); tp->t_pgrp = pgrp; break; } @@ -1527,7 +1526,7 @@ ttymodem(tp, flag) SET(tp->t_state, TS_ZOMBIE); CLR(tp->t_state, TS_CONNECTED); if (tp->t_session) { - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); if (tp->t_session->s_leader) { struct proc *p; @@ -1536,7 +1535,7 @@ ttymodem(tp, flag) psignal(p, SIGHUP); PROC_UNLOCK(p); } - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); } ttyflush(tp, FREAD | FWRITE); return (0); @@ -1601,6 +1600,7 @@ ttread(tp, uio, flag) int has_stime = 0, last_cc = 0; long slp = 0; /* XXX this should be renamed `timo'. */ struct timeval stime; + struct pgrp *pg; loop: s = spltty(); @@ -1620,20 +1620,21 @@ ttread(tp, uio, flag) */ if (isbackground(p, tp)) { splx(s); - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); PROC_LOCK(p); if (SIGISMEMBER(p->p_sigignore, SIGTTIN) || SIGISMEMBER(p->p_sigmask, SIGTTIN) || (p->p_flag & P_PPWAIT) || p->p_pgrp->pg_jobc == 0) { PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (EIO); } + pg = p->p_pgrp; PROC_UNLOCK(p); - PGRP_LOCK(p->p_pgrp); - PGRPSESS_SUNLOCK(); - pgsignal(p->p_pgrp, SIGTTIN, 1); - PGRP_UNLOCK(p->p_pgrp); + PGRP_LOCK(pg); + sx_sunlock(&proctree_lock); + pgsignal(pg, SIGTTIN, 1); + PGRP_UNLOCK(pg); error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, "ttybg2", 0); if (error) return (error); @@ -1939,7 +1940,7 @@ ttwrite(tp, uio, flag) * Hang the process if it's in the background. */ p = curproc; - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); PROC_LOCK(p); if (isbackground(p, tp) && ISSET(tp->t_lflag, TOSTOP) && !(p->p_flag & P_PPWAIT) && @@ -1947,13 +1948,13 @@ ttwrite(tp, uio, flag) !SIGISMEMBER(p->p_sigmask, SIGTTOU)) { if (p->p_pgrp->pg_jobc == 0) { PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); error = EIO; goto out; } PROC_UNLOCK(p); PGRP_LOCK(p->p_pgrp); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); pgsignal(p->p_pgrp, SIGTTOU, 1); PGRP_UNLOCK(p->p_pgrp); error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, "ttybg4", 0); @@ -1962,7 +1963,7 @@ ttwrite(tp, uio, flag) goto loop; } else { PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); } /* * Process the user's data in at most OBUFSIZ chunks. Perform any diff --git a/sys/kern/tty_pty.c b/sys/kern/tty_pty.c index 9bc236068448..7d6e736b3b95 100644 --- a/sys/kern/tty_pty.c +++ b/sys/kern/tty_pty.c @@ -234,25 +234,27 @@ ptsread(dev, uio, flag) struct proc *p = td->td_proc; register struct tty *tp = dev->si_tty; register struct pt_ioctl *pti = dev->si_drv1; + struct pgrp *pg; int error = 0; again: if (pti->pt_flags & PF_REMOTE) { while (isbackground(p, tp)) { - PGRPSESS_SLOCK(); + sx_slock(&proctree_lock); PROC_LOCK(p); if (SIGISMEMBER(p->p_sigignore, SIGTTIN) || SIGISMEMBER(p->p_sigmask, SIGTTIN) || p->p_pgrp->pg_jobc == 0 || p->p_flag & P_PPWAIT) { PROC_UNLOCK(p); - PGRPSESS_SUNLOCK(); + sx_sunlock(&proctree_lock); return (EIO); } + pg = p->p_pgrp; PROC_UNLOCK(p); - PGRP_LOCK(p->p_pgrp); - PGRPSESS_SUNLOCK(); - pgsignal(p->p_pgrp, SIGTTIN, 1); - PGRP_UNLOCK(p->p_pgrp); + PGRP_LOCK(pg); + sx_sunlock(&proctree_lock); + pgsignal(pg, SIGTTIN, 1); + PGRP_UNLOCK(pg); error = ttysleep(tp, &lbolt, TTIPRI | PCATCH, "ptsbg", 0); if (error)