Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 23 Oct 2009 15:14:54 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r198411 - in head/sys: kern sys
Message-ID:  <200910231514.n9NFEsfW064873@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: jhb
Date: Fri Oct 23 15:14:54 2009
New Revision: 198411
URL: http://svn.freebsd.org/changeset/base/198411

Log:
  - Fix several off-by-one errors when using MAXCOMLEN.  The p_comm[] and
    td_name[] arrays are actually MAXCOMLEN + 1 in size and a few places that
    created shadow copies of these arrays were just using MAXCOMLEN.
  - Prefer using sizeof() of an array type to explicit constants for the
    array length in a few places.
  - Ensure that all of p_comm[] and td_name[] is always zero'd during
    execve() to guard against any possible information leaks.  Previously
    trailing garbage in p_comm[] could be leaked to userland in ktrace
    record headers via td_name[].
  
  Reviewed by:	bde

Modified:
  head/sys/kern/kern_exec.c
  head/sys/kern/kern_ktrace.c
  head/sys/kern/subr_bus.c
  head/sys/kern/subr_taskqueue.c
  head/sys/sys/interrupt.h

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c	Fri Oct 23 15:12:05 2009	(r198410)
+++ head/sys/kern/kern_exec.c	Fri Oct 23 15:14:54 2009	(r198411)
@@ -326,7 +326,7 @@ do_execve(td, args, mac_p)
 	struct ucred *newcred = NULL, *oldcred;
 	struct uidinfo *euip;
 	register_t *stack_base;
-	int error, len = 0, i;
+	int error, i;
 	struct image_params image_params, *imgp;
 	struct vattr attr;
 	int (*img_first)(struct image_params *);
@@ -602,18 +602,12 @@ interpret:
 	execsigs(p);
 
 	/* name this process - nameiexec(p, ndp) */
-	if (args->fname) {
-		len = min(nd.ni_cnd.cn_namelen,MAXCOMLEN);
-		bcopy(nd.ni_cnd.cn_nameptr, p->p_comm, len);
-	} else {
-		if (vn_commname(binvp, p->p_comm, MAXCOMLEN + 1) == 0)
-			len = MAXCOMLEN;
-		else {
-			len = sizeof(fexecv_proc_title);
-			bcopy(fexecv_proc_title, p->p_comm, len);
-		}
-	}
-	p->p_comm[len] = 0;
+	bzero(p->p_comm, sizeof(p->p_comm));
+	if (args->fname)
+		bcopy(nd.ni_cnd.cn_nameptr, p->p_comm,
+		    min(nd.ni_cnd.cn_namelen, MAXCOMLEN));
+	else if (vn_commname(binvp, p->p_comm, sizeof(p->p_comm)) != 0)
+		bcopy(fexecv_proc_title, p->p_comm, sizeof(fexecv_proc_title));
 	bcopy(p->p_comm, td->td_name, sizeof(td->td_name));
 
 	/*

Modified: head/sys/kern/kern_ktrace.c
==============================================================================
--- head/sys/kern/kern_ktrace.c	Fri Oct 23 15:12:05 2009	(r198410)
+++ head/sys/kern/kern_ktrace.c	Fri Oct 23 15:14:54 2009	(r198411)
@@ -256,6 +256,10 @@ ktrace_resize_pool(u_int newsize)
 	return (ktr_requestpool);
 }
 
+/* ktr_getrequest() assumes that ktr_comm[] is the same size as td_name[]. */
+CTASSERT(sizeof(((struct ktr_header *)NULL)->ktr_comm) ==
+    (sizeof((struct thread *)NULL)->td_name));
+
 static struct ktr_request *
 ktr_getrequest(int type)
 {
@@ -283,7 +287,8 @@ ktr_getrequest(int type)
 		microtime(&req->ktr_header.ktr_time);
 		req->ktr_header.ktr_pid = p->p_pid;
 		req->ktr_header.ktr_tid = td->td_tid;
-		bcopy(td->td_name, req->ktr_header.ktr_comm, MAXCOMLEN + 1);
+		bcopy(td->td_name, req->ktr_header.ktr_comm,
+		    sizeof(req->ktr_header.ktr_comm));
 		req->ktr_buffer = NULL;
 		req->ktr_header.ktr_len = 0;
 	} else {

Modified: head/sys/kern/subr_bus.c
==============================================================================
--- head/sys/kern/subr_bus.c	Fri Oct 23 15:12:05 2009	(r198410)
+++ head/sys/kern/subr_bus.c	Fri Oct 23 15:14:54 2009	(r198411)
@@ -3861,8 +3861,8 @@ int
 bus_describe_intr(device_t dev, struct resource *irq, void *cookie,
     const char *fmt, ...)
 {
-	char descr[MAXCOMLEN];
 	va_list ap;
+	char descr[MAXCOMLEN + 1];
 
 	if (dev->parent == NULL)
 		return (EINVAL);

Modified: head/sys/kern/subr_taskqueue.c
==============================================================================
--- head/sys/kern/subr_taskqueue.c	Fri Oct 23 15:12:05 2009	(r198410)
+++ head/sys/kern/subr_taskqueue.c	Fri Oct 23 15:14:54 2009	(r198411)
@@ -301,7 +301,7 @@ taskqueue_start_threads(struct taskqueue
 	struct thread *td;
 	struct taskqueue *tq;
 	int i, error;
-	char ktname[MAXCOMLEN];
+	char ktname[MAXCOMLEN + 1];
 
 	if (count <= 0)
 		return (EINVAL);
@@ -309,7 +309,7 @@ taskqueue_start_threads(struct taskqueue
 	tq = *tqp;
 
 	va_start(ap, name);
-	vsnprintf(ktname, MAXCOMLEN, name, ap);
+	vsnprintf(ktname, sizeof(ktname), name, ap);
 	va_end(ap);
 
 	tq->tq_threads = malloc(sizeof(struct thread *) * count, M_TASKQUEUE,

Modified: head/sys/sys/interrupt.h
==============================================================================
--- head/sys/sys/interrupt.h	Fri Oct 23 15:12:05 2009	(r198410)
+++ head/sys/sys/interrupt.h	Fri Oct 23 15:14:54 2009	(r198411)
@@ -47,7 +47,7 @@ struct intr_handler {
 	driver_intr_t	*ih_handler;	/* Threaded handler function. */
 	void		*ih_argument;	/* Argument to pass to handlers. */
 	int		 ih_flags;
-	char		 ih_name[MAXCOMLEN]; /* Name of handler. */
+	char		 ih_name[MAXCOMLEN + 1]; /* Name of handler. */
 	struct intr_event *ih_event;	/* Event we are connected to. */
 	int		 ih_need;	/* Needs service. */
 	TAILQ_ENTRY(intr_handler) ih_next; /* Next handler for this event. */
@@ -104,8 +104,8 @@ struct intr_handler {
 struct intr_event {
 	TAILQ_ENTRY(intr_event) ie_list;
 	TAILQ_HEAD(, intr_handler) ie_handlers; /* Interrupt handlers. */
-	char		ie_name[MAXCOMLEN]; /* Individual event name. */
-	char		ie_fullname[MAXCOMLEN];
+	char		ie_name[MAXCOMLEN + 1]; /* Individual event name. */
+	char		ie_fullname[MAXCOMLEN + 1];
 	struct mtx	ie_lock;
 	void		*ie_source;	/* Cookie used by MD code. */
 	struct intr_thread *ie_thread;	/* Thread we are connected to. */



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