From 8b90699bbb47e00e66db06c02537f5e3e9d77982 Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Mon, 13 Sep 2004 16:22:41 +0000 Subject: [PATCH] Clean up struct session hold/rele management. The tty half-closed support (i.e. showing 'p0-' in the ps output for the tty instead of '??' after a process has detached) had an issue where the tty would be left with a reference to the freed session structure in certain situations because the session structure's ref-counting code was not properly implementing the release case. Consolidate the disparate session ref-counting code into real sess_hold() and sess_rele() functions and ensure that any tty reference to the session is cleared before the session structure is free()'d. NOTE: Joerg noticed a 0xdeadc1de (deadcode) panic related to this issue which means that prior to this fix it was possible for the bug to cause memory corruption in certain situations. NOTE: Linux does not implement half-closed tty sessions like BSD. Add code to implement fully-closed tty sessions, and document the whole mess, but leave it conditionalized-out for now. Reported-by: Joerg Sonnenberger --- sys/kern/kern_proc.c | 40 +++++++++++++++++++++++++++++++++++----- sys/kern/subr_prf.c | 6 +++--- sys/kern/tty.c | 42 ++++++++++++++++++++++++++++++++++++++---- sys/kern/tty_cons.c | 5 ++--- sys/sys/proc.h | 9 +++------ sys/sys/tty.h | 3 ++- 6 files changed, 83 insertions(+), 22 deletions(-) diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c index e47be18eee..5ca1d8a786 100644 --- a/sys/kern/kern_proc.c +++ b/sys/kern/kern_proc.c @@ -32,7 +32,7 @@ * * @(#)kern_proc.c 8.7 (Berkeley) 2/14/95 * $FreeBSD: src/sys/kern/kern_proc.c,v 1.63.2.9 2003/05/08 07:47:16 kbyanc Exp $ - * $DragonFly: src/sys/kern/kern_proc.c,v 1.16 2004/07/24 20:21:35 dillon Exp $ + * $DragonFly: src/sys/kern/kern_proc.c,v 1.17 2004/09/13 16:22:36 dillon Exp $ */ #include @@ -189,7 +189,7 @@ enterpgrp(p, pgid, mksess) ("enterpgrp: mksession and p != curproc")); } else { pgrp->pg_session = p->p_session; - pgrp->pg_session->s_count++; + sess_hold(pgrp->pg_session); } pgrp->pg_id = pgid; LIST_INIT(&pgrp->pg_members); @@ -248,9 +248,39 @@ pgdelete(pgrp) pgrp->pg_session->s_ttyp->t_pgrp == pgrp) pgrp->pg_session->s_ttyp->t_pgrp = NULL; LIST_REMOVE(pgrp, pg_hash); - if (--pgrp->pg_session->s_count == 0) - FREE(pgrp->pg_session, M_SESSION); - FREE(pgrp, M_PGRP); + sess_rele(pgrp->pg_session); + free(pgrp, M_PGRP); +} + +/* + * Adjust the ref count on a session structure. When the ref count falls to + * zero the tty is disassociated from the session and the session structure + * is freed. Note that tty assocation is not itself ref-counted. + */ +void +sess_hold(struct session *sp) +{ + ++sp->s_count; +} + +void +sess_rele(struct session *sp) +{ + KKASSERT(sp->s_count > 0); + if (--sp->s_count == 0) { + if (sp->s_ttyp && sp->s_ttyp->t_session) { +#ifdef TTY_DO_FULL_CLOSE + /* FULL CLOSE, see ttyclearsession() */ + KKASSERT(sp->s_ttyp->t_session == sp); + sp->s_ttyp->t_session = NULL; +#else + /* HALF CLOSE, see ttyclearsession() */ + if (sp->s_ttyp->t_session == sp) + sp->s_ttyp->t_session = NULL; +#endif + } + free(sp, M_SESSION); + } } /* diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c index 1d9ccb8989..a1df486037 100644 --- a/sys/kern/subr_prf.c +++ b/sys/kern/subr_prf.c @@ -37,7 +37,7 @@ * * @(#)subr_prf.c 8.3 (Berkeley) 1/21/94 * $FreeBSD: src/sys/kern/subr_prf.c,v 1.61.2.5 2002/08/31 18:22:08 dwmalone Exp $ - * $DragonFly: src/sys/kern/subr_prf.c,v 1.7 2003/11/09 02:22:36 dillon Exp $ + * $DragonFly: src/sys/kern/subr_prf.c,v 1.8 2004/09/13 16:22:36 dillon Exp $ */ #include @@ -140,7 +140,7 @@ tprintf_open(struct proc *p) { if ((p->p_flag & P_CONTROLT) && p->p_session->s_ttyvp) { - SESSHOLD(p->p_session); + sess_hold(p->p_session); return ((tpr_t) p->p_session); } return ((tpr_t) NULL); @@ -150,7 +150,7 @@ void tprintf_close(tpr_t sess) { if (sess) - SESSRELE((struct session *) sess); + sess_rele((struct session *) sess); } /* diff --git a/sys/kern/tty.c b/sys/kern/tty.c index 7ef767be95..4659abe107 100644 --- a/sys/kern/tty.c +++ b/sys/kern/tty.c @@ -37,7 +37,7 @@ * * @(#)tty.c 8.8 (Berkeley) 1/21/94 * $FreeBSD: src/sys/kern/tty.c,v 1.129.2.5 2002/03/11 01:32:31 dd Exp $ - * $DragonFly: src/sys/kern/tty.c,v 1.11 2004/05/17 07:12:31 dillon Exp $ + * $DragonFly: src/sys/kern/tty.c,v 1.12 2004/09/13 16:22:36 dillon Exp $ */ /*- @@ -229,6 +229,7 @@ ttyopen(device, tp) * Handle close() on a tty line: flush and set to initial state, * bumping generation number so that pending read/write calls * can detect recycling of the tty. + * * XXX our caller should have done `spltty(); l_close(); ttyclose();' * and l_close() should have flushed, but we repeat the spltty() and * the flush in case there are buggy callers. @@ -251,13 +252,42 @@ ttyclose(tp) tp->t_gen++; tp->t_line = TTYDISC; - tp->t_pgrp = NULL; - tp->t_session = NULL; + ttyclearsession(tp); tp->t_state = 0; splx(s); return (0); } +/* + * Disassociate the tty from its session. Traditionally this has only been + * a half-close, meaning that the session was still allowed to point at the + * tty (resulting in the tty in the ps command showing something like 'p0-'), + * even though the tty is no longer pointing at the session. + * + * The half close seems to be useful only for 'ps' output but there is as + * yet no reason to remove the feature. The full-close code is currently + * #if 0'd out. See also sess_rele() in kern/kern_proc.c. + */ +void +ttyclearsession(struct tty *tp) +{ + struct session *sp; + + tp->t_pgrp = NULL; + if ((sp = tp->t_session) != NULL) { + tp->t_session = NULL; +#ifdef TTY_DO_FULL_CLOSE + /* FULL CLOSE (not yet) */ + if (sp->s_ttyp == tp) { + sp->s_ttyp = NULL; + } else { + printf("ttyclearsession: warning: sp->s_ttyp != tp " + "%p/%p\n", sp->s_ttyp, tp); + } +#endif + } +} + #define FLUSHQ(q) { \ if ((q)->c_cc) \ ndflush(q, (q)->c_cc); \ @@ -2530,10 +2560,14 @@ ttymalloc(tp) return (tp); } -#if 0 /* XXX not yet usable: session leader holds a ref (see kern_exit.c). */ +#if 0 /* * Free a tty struct. Clists in the struct should have been freed by * ttyclose(). + * + * XXX not yet usable: since we support a half-closed state and do not + * ref count the tty reference from the session, it is possible for a + * session to hold a ref on the tty. See TTY_DO_FULL_CLOSE. */ void ttyfree(tp) diff --git a/sys/kern/tty_cons.c b/sys/kern/tty_cons.c index 40a7bf6920..db1537b127 100644 --- a/sys/kern/tty_cons.c +++ b/sys/kern/tty_cons.c @@ -37,7 +37,7 @@ * * from: @(#)cons.c 7.2 (Berkeley) 5/9/91 * $FreeBSD: src/sys/kern/tty_cons.c,v 1.81.2.4 2001/12/17 18:44:41 guido Exp $ - * $DragonFly: src/sys/kern/tty_cons.c,v 1.13 2004/05/19 22:52:58 dillon Exp $ + * $DragonFly: src/sys/kern/tty_cons.c,v 1.14 2004/09/13 16:22:36 dillon Exp $ */ #include "opt_ddb.h" @@ -420,8 +420,7 @@ cnclose(struct cdevmsg_close *msg) if (cn_tp) { /* perform a ttyhalfclose() */ /* reset session and proc group */ - cn_tp->t_pgrp = NULL; - cn_tp->t_session = NULL; + ttyclearsession(cn_tp); } return (0); } diff --git a/sys/sys/proc.h b/sys/sys/proc.h index ca1db50a7c..eac7c488fe 100644 --- a/sys/sys/proc.h +++ b/sys/sys/proc.h @@ -37,7 +37,7 @@ * * @(#)proc.h 8.15 (Berkeley) 5/19/95 * $FreeBSD: src/sys/sys/proc.h,v 1.99.2.9 2003/06/06 20:21:32 tegge Exp $ - * $DragonFly: src/sys/sys/proc.h,v 1.54 2004/08/12 19:59:30 eirikn Exp $ + * $DragonFly: src/sys/sys/proc.h,v 1.55 2004/09/13 16:22:41 dillon Exp $ */ #ifndef _SYS_PROC_H_ @@ -329,11 +329,6 @@ MALLOC_DECLARE(M_PARGS); #define NO_PID 100000 #define SESS_LEADER(p) ((p)->p_session->s_leader == (p)) -#define SESSHOLD(s) ((s)->s_count++) -#define SESSRELE(s) { \ - if (--(s)->s_count == 0) \ - FREE(s, M_SESSION); \ -} /* * STOPEVENT @@ -430,6 +425,8 @@ int enterpgrp (struct proc *p, pid_t pgid, int mksess); void fixjobc (struct proc *p, struct pgrp *pgrp, int entering); int inferior (struct proc *p); int leavepgrp (struct proc *p); +void sess_hold(struct session *sp); +void sess_rele(struct session *sp); void mi_switch (struct proc *p); void procinit (void); void relscurproc(struct proc *curp); diff --git a/sys/sys/tty.h b/sys/sys/tty.h index b2ad72b376..c55c12e30e 100644 --- a/sys/sys/tty.h +++ b/sys/sys/tty.h @@ -37,7 +37,7 @@ * * @(#)tty.h 8.6 (Berkeley) 1/21/94 * $FreeBSD: src/sys/sys/tty.h,v 1.53.2.1 2001/02/26 04:23:21 jlemon Exp $ - * $DragonFly: src/sys/sys/tty.h,v 1.5 2003/08/20 07:31:21 rob Exp $ + * $DragonFly: src/sys/sys/tty.h,v 1.6 2004/09/13 16:22:41 dillon Exp $ */ #ifndef _SYS_TTY_H_ @@ -252,6 +252,7 @@ void ttyblock (struct tty *tp); void ttychars (struct tty *tp); int ttycheckoutq (struct tty *tp, int wait); int ttyclose (struct tty *tp); +void ttyclearsession (struct tty *tp); void ttyflush (struct tty *tp, int rw); void ttyfree (struct tty *tp); void ttyinfo (struct tty *tp); -- 2.41.0