Clean up struct session hold/rele management. The tty half-closed support
authorMatthew Dillon <dillon@dragonflybsd.org>
Mon, 13 Sep 2004 16:22:41 +0000 (16:22 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Mon, 13 Sep 2004 16:22:41 +0000 (16:22 +0000)
(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 <joerg@britannica.bec.de>
sys/kern/kern_proc.c
sys/kern/subr_prf.c
sys/kern/tty.c
sys/kern/tty_cons.c
sys/sys/proc.h
sys/sys/tty.h

index e47be18..5ca1d8a 100644 (file)
@@ -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 <sys/param.h>
@@ -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);
+       }
 }
 
 /*
index 1d9ccb8..a1df486 100644 (file)
@@ -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 <sys/param.h>
@@ -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);
 }
 
 /*
index 7ef767b..4659abe 100644 (file)
@@ -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)
index 40a7bf6..db1537b 100644 (file)
@@ -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);
                }
index ca1db50..eac7c48 100644 (file)
@@ -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);
index b2ad72b..c55c12e 100644 (file)
@@ -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);