- 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
This commit is contained in:
John Baldwin 2009-10-23 15:14:54 +00:00
parent 0deb032554
commit 5ca4819ddf
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=198411
5 changed files with 19 additions and 20 deletions

View File

@ -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 @@ do_execve(td, args, mac_p)
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));
/*

View File

@ -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 {

View File

@ -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);

View File

@ -301,7 +301,7 @@ taskqueue_start_threads(struct taskqueue **tqp, int count, int pri,
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 **tqp, int count, int pri,
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,

View File

@ -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. */