kern/80742: [PATCH] Local DoS in sys/compat/pecoff (+ other fixes)
Wojciech A. Koszek
dunstan at freebsd.czest.pl
Sat May 7 17:00:20 PDT 2005
>Number: 80742
>Category: kern
>Synopsis: [PATCH] Local DoS in sys/compat/pecoff (+ other fixes)
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun May 08 00:00:19 GMT 2005
>Closed-Date:
>Last-Modified:
>Originator: Wojciech A. Koszek
>Release: FreeBSD 5.4-STABLE i386
>Organization:
>Environment:
System: FreeBSD dunstan.freebsd.czest.pl 5.4-STABLE FreeBSD 5.4-STABLE #1: Sat Apr 30 00:07:09 CEST 2005 root at dunstan.freebsd.czest.pl:/usr/obj/usr/src/sys/HOME8 i386
>Description:
I've found problem in sys/compat/pecoff sources which crashes machine.
It's related with handling content of PECOFF header in imgact_pecoff.c.
PECOFF header contains: magic number (used by kernel code to direct content
of executable file to proper routines), 0x3a bytes of stub data (unused in
compatibility code) and offset to additional structures. Setting last
variable to negative values causes kernel panic, since it's passed
unvalidated to vn_rdwr function through pecoff_read_from(). Kernel crashes
in ffs_read (negative offset in uio structure). Please refer to PECOFF
standard and sys/compat/pecoff in order to confirm existanse of bug
described in this report. Did anyone find PECOFF code useful?
>How-To-Repeat:
kldload /boot/kernel/pecoff.ko
<pecoff_crash.pl>
#!/usr/local/bin/perl
my $p = 0x3a;
print "MZ" . "x"x$p . pack("l", -100);
</pecoff_crash.pl>
./pecoff_crash > pecoff
chmod 755 pecoff
./pecoff
>Fix:
Specification doesn't remind about possibility of negative values of PEOFS, so this
variable might be declared as "unsigned". My patch contains also:
- fix for bug
- style(9) fixes/consistent malloc checking
- proper error handling
- DPRINTF from .c file -> PE_DEBUG in .h file with more readable output.
- DEBUG -> PECOFF_DEBUG, since debugging code is enabled when PECOFF_DEBUG
is defined (not DEBUG)
--- diff.0.pecoff begins here ---
Patch against FreeBSD 5.4-STABLE, kern.osreldate: 504100.
diff -uprP /usr/src/sys/compat/pecoff/imgact_pecoff.c src/sys/compat/pecoff/imgact_pecoff.c
--- /usr/src/sys/compat/pecoff/imgact_pecoff.c Sat May 7 15:11:12 2005
+++ src/sys/compat/pecoff/imgact_pecoff.c Sat May 7 17:58:09 2005
@@ -78,13 +78,7 @@ __FBSDID("$FreeBSD: src/sys/compat/pecof
#include "opt_pecoff.h"
-#define PECOFF_PE_SIGNATURE "PE\0\0"
static int pecoff_fixup(register_t **, struct image_params *);
-#ifndef PECOFF_DEBUG
-#define DPRINTF(a)
-#else
-#define DPRINTF(a) printf a
-#endif
static struct sysentvec pecoff_sysvec = {
SYS_MAXSYSCALL,
sysent,
@@ -129,31 +123,33 @@ static int
exec_pecoff_coff_prep_zmagic(struct image_params *,
struct coff_filehdr *,
struct coff_aouthdr *, int peoffs);
-
static int
exec_pecoff_coff_makecmds(struct image_params *,
struct coff_filehdr *, int);
-
-static int pecoff_signature(struct thread *, struct vnode *, const struct pecoff_dos_filehdr *);
-static int pecoff_read_from(struct thread *, struct vnode *, int, caddr_t, int);
+static int
+pecoff_signature(struct thread *, struct vnode *,
+ const struct pecoff_dos_filehdr *);
+static int
+pecoff_read_from(struct thread *, struct vnode *,
+ unsigned int, caddr_t, int);
+static int
+pecoff_load_section(struct thread *, struct vmspace *, struct vnode *,
+ vm_offset_t, caddr_t, size_t, size_t, vm_prot_t);
static int
-pecoff_load_section(struct thread * td,
- struct vmspace * vmspace, struct vnode * vp,
- vm_offset_t offset, caddr_t vmaddr, size_t memsz, size_t filsz,
- vm_prot_t prot);
+pecoff_load_file(struct thread *, const char *, u_long *, u_long *,
+ u_long *);
static int
-pecoff_fixup(register_t ** stack_base, struct image_params * imgp)
+pecoff_fixup(register_t **stack_base, struct image_params *imgp)
{
- int len = sizeof(struct pecoff_args);
+ int len = sizeof(struct pecoff_args);
struct pecoff_imghdr *ap;
- register_t *pos;
+ register_t *pos;
pos = *stack_base + (imgp->argc + imgp->envc + 2);
ap = (struct pecoff_imghdr *) imgp->auxargs;
- if (copyout(ap, pos, len)) {
+ if (copyout(ap, pos, len))
return 0;
- }
free(ap, M_TEMP);
imgp->auxargs = NULL;
(*stack_base)--;
@@ -162,25 +158,34 @@ pecoff_fixup(register_t ** stack_base, s
}
static int
-pecoff_load_section(struct thread * td, struct vmspace * vmspace, struct vnode * vp, vm_offset_t offset, caddr_t vmaddr, size_t memsz, size_t filsz, vm_prot_t prot)
+pecoff_load_section(td, vmspace, vp, offset, vmaddr, memsz, filsz, prot)
+ struct thread *td;
+ struct vmspace *vmspace;
+ struct vnode *vp;
+ vm_offset_t offset;
+ caddr_t vmaddr;
+ size_t memsz;
+ size_t filsz;
+ vm_prot_t prot;
{
- size_t map_len;
- vm_offset_t map_addr;
- int error, rv;
- size_t copy_len;
- size_t copy_map_len;
- size_t copy_start;
- vm_object_t object;
- vm_offset_t copy_map_offset;
- vm_offset_t file_addr;
- vm_offset_t data_buf = 0;
+ size_t map_len;
+ vm_offset_t map_addr;
+ int error, rv;
+ size_t copy_len;
+ size_t copy_map_len;
+ size_t copy_start;
+ vm_object_t object;
+ vm_offset_t copy_map_offset;
+ vm_offset_t file_addr;
+ vm_offset_t data_buf = 0;
object = vp->v_object;
error = 0;
map_addr = trunc_page((vm_offset_t) vmaddr);
file_addr = trunc_page(offset);
- DPRINTF(("SECARG:%x %p %x %x\n", offset, vmaddr, memsz, filsz));
+ PE_DEBUG("Section args (offset=0x%x, vmaddr: %p, memsz: 0x%x, "
+ "filsz: 0x%x)", offset, vmaddr, memsz, filsz);
if (file_addr != offset) {
/*
* The section is not on page boundary. We can't use
@@ -192,12 +197,12 @@ pecoff_load_section(struct thread * td,
copy_map_len = round_page(offset + filsz) - file_addr;
copy_start = offset - file_addr;
- DPRINTF(("offset=%x vmaddr=%lx filsz=%x memsz=%x\n",
- offset, (long)vmaddr, filsz, memsz));
- DPRINTF(("map_len=%x copy_len=%x copy_map_offset=%x"
- " copy_map_len=%x copy_start=%x\n",
+ PE_DEBUG("offset=0x%x vmaddr=0x%lx filsz=0x%x memsz=0x%x",
+ offset, (long)vmaddr, filsz, memsz);
+ PE_DEBUG("map_len=0x%x copy_len=0x%x copy_map_offset=0x%x"
+ " copy_map_len=0x%x copy_start=0x%x",
map_len, copy_len, copy_map_offset,
- copy_map_len, copy_start));
+ copy_map_len, copy_start);
} else {
map_len = trunc_page(filsz);
@@ -239,12 +244,14 @@ pecoff_load_section(struct thread * td,
map_addr, map_addr + map_len,
VM_PROT_ALL, VM_PROT_ALL, 0);
vm_map_unlock(&vmspace->vm_map);
- DPRINTF(("EMP-rv:%d,%x %x\n", rv, map_addr, map_addr + map_len));
+ PE_DEBUG("mmap error=%d, mapaddr=0x%x, "
+ "mapaddr + maplen=0x%x", rv, map_addr,
+ map_addr + map_len);
if (rv != KERN_SUCCESS) {
return EINVAL;
}
}
- DPRINTF(("COPYARG %x %x\n", map_addr, copy_len));
+ PE_DEBUG("Copying arguments from 0x%x, length=0x%x", map_addr, copy_len);
if (copy_len != 0) {
vm_object_reference(object);
rv = vm_map_find(exec_map,
@@ -265,7 +272,7 @@ pecoff_load_section(struct thread * td,
error = copyout((caddr_t) data_buf + copy_start,
(caddr_t) map_addr, copy_len);
vm_map_remove(exec_map, data_buf, data_buf + copy_map_len);
- DPRINTF(("%d\n", error));
+ PE_DEBUG("copyout()=%d", error);
if (error)
return (error);
}
@@ -278,21 +285,27 @@ pecoff_load_section(struct thread * td,
return error;
}
+
static int
-pecoff_load_file(struct thread * td, const char *file, u_long * addr, u_long * entry, u_long * ldexport)
+pecoff_load_file(td, file, addr, entry, ldexport)
+ struct thread *td;
+ const char *file;
+ u_long *addr;
+ u_long *entry;
+ u_long *ldexport;
{
struct nameidata nd;
struct pecoff_dos_filehdr dh;
- struct coff_filehdr *fp = 0;
+ struct coff_filehdr *fp = NULL;
struct coff_aouthdr *ap;
struct pecoff_opthdr *wp;
- struct coff_scnhdr *sh = 0;
+ struct coff_scnhdr *sh = NULL;
struct vmspace *vmspace = td->td_proc->p_vmspace;
- struct vattr attr;
+ struct vattr attr;
struct image_params image_params, *imgp;
- int peofs;
- int error, i, scnsiz;
+ unsigned int peofs;
+ int error, i, scnsiz;
imgp = &image_params;
/*
@@ -306,7 +319,6 @@ pecoff_load_file(struct thread * td, con
imgp->firstpage = NULL;
NDINIT(&nd, LOOKUP, LOCKLEAF | FOLLOW, UIO_SYSSPACE, file, td);
-
if ((error = namei(&nd)) != 0) {
nd.ni_vp = NULL;
goto fail;
@@ -325,7 +337,7 @@ pecoff_load_file(struct thread * td, con
VOP_UNLOCK(nd.ni_vp, 0, td);
if (error)
goto fail;
- if ((error = pecoff_read_from(td, imgp->vp, 0, (caddr_t) & dh, sizeof(dh))) != 0)
+ if ((error = pecoff_read_from(td, imgp->vp, 0, (caddr_t) &dh, sizeof(dh))) != 0)
goto fail;
if ((error = pecoff_signature(td, imgp->vp, &dh) != 0))
goto fail;
@@ -342,6 +354,10 @@ pecoff_load_file(struct thread * td, con
/* read section header */
scnsiz = sizeof(struct coff_scnhdr) * fp->f_nscns;
sh = malloc(scnsiz, M_TEMP, M_WAITOK);
+ if (sh == NULL) {
+ error = ENOMEM;
+ goto fail;
+ }
if ((error = pecoff_read_from(td, imgp->vp, peofs + PECOFF_HDR_SIZE,
(caddr_t) sh, scnsiz)) != 0)
goto fail;
@@ -367,10 +383,9 @@ pecoff_load_file(struct thread * td, con
prot |= (sh[i].s_flags & COFF_STYP_EXEC) ? VM_PROT_EXECUTE : 0;
sh[i].s_vaddr += wp->w_base; /* RVA --> VA */
- if ((error = pecoff_load_section(td, vmspace, imgp->vp, sh[i].s_scnptr
- ,(caddr_t) sh[i].s_vaddr,
- sh[i].s_paddr, sh[i].s_size
- ,prot)) != 0)
+ if ((error = pecoff_load_section(td, vmspace, imgp->vp, sh[i].s_scnptr,
+ (caddr_t) sh[i].s_vaddr,
+ sh[i].s_paddr, sh[i].s_size, prot)) != 0)
goto fail;
}
@@ -378,46 +393,48 @@ pecoff_load_file(struct thread * td, con
*addr = wp->w_base;
*ldexport = wp->w_imghdr[0].i_vaddr + wp->w_base;
fail:
- if (fp)
+ if (fp != NULL)
free(fp, M_TEMP);
- if (sh)
+ if (sh != NULL)
free(sh, M_TEMP);
- if (nd.ni_vp)
+ if (nd.ni_vp != NULL)
vrele(nd.ni_vp);
return error;
}
static int
-exec_pecoff_coff_prep_omagic(struct image_params * imgp,
- struct coff_filehdr * fp,
- struct coff_aouthdr * ap, int peofs)
+exec_pecoff_coff_prep_omagic(struct image_params *imgp,
+ struct coff_filehdr *fp,
+ struct coff_aouthdr *ap, int peofs)
{
return ENOEXEC;
}
static int
-exec_pecoff_coff_prep_nmagic(struct image_params * imgp,
- struct coff_filehdr * fp,
- struct coff_aouthdr * ap, int peofs)
+exec_pecoff_coff_prep_nmagic(struct image_params *imgp,
+ struct coff_filehdr *fp,
+ struct coff_aouthdr *ap, int peofs)
{
return ENOEXEC;
}
static int
-exec_pecoff_coff_prep_zmagic(struct image_params * imgp,
- struct coff_filehdr * fp,
- struct coff_aouthdr * ap, int peofs)
+exec_pecoff_coff_prep_zmagic(struct image_params *imgp,
+ struct coff_filehdr *fp,
+ struct coff_aouthdr *ap, int peofs)
{
- int scnsiz = sizeof(struct coff_scnhdr) * fp->f_nscns;
- int error = ENOEXEC, i;
- int prot;
- u_long text_size = 0, data_size = 0, dsize;
- u_long text_addr = 0, data_addr = VM_MAXUSER_ADDRESS;
- u_long ldexport, ldbase;
+ int scnsiz = sizeof(struct coff_scnhdr) * fp->f_nscns;
+ int error = ENOEXEC, i;
+ int prot;
+ u_long text_size = 0, data_size = 0, dsize;
+ u_long text_addr = 0, data_addr = VM_MAXUSER_ADDRESS;
+ u_long ldexport, ldbase;
struct pecoff_opthdr *wp;
struct coff_scnhdr *sh;
struct vmspace *vmspace;
struct pecoff_args *argp = NULL;
sh = malloc(scnsiz, M_TEMP, M_WAITOK);
+ if (sh == NULL)
+ return ENOMEM;
wp = (void *) ((char *) ap + sizeof(struct coff_aouthdr));
error = pecoff_read_from(FIRST_THREAD_IN_PROC(imgp->proc), imgp->vp,
@@ -435,13 +452,12 @@ exec_pecoff_coff_prep_zmagic(struct imag
if (sh[i].s_flags & COFF_STYP_DISCARD)
continue;
if ((sh[i].s_flags & COFF_STYP_TEXT) != 0) {
-
error = pecoff_load_section(
FIRST_THREAD_IN_PROC(imgp->proc),
vmspace, imgp->vp, sh[i].s_scnptr,
(caddr_t) sh[i].s_vaddr, sh[i].s_paddr,
sh[i].s_size ,prot);
- DPRINTF(("ERROR%d\n", error));
+ PE_DEBUG("pecoff_load_section() returned %d", error);
if (error)
goto fail;
text_addr = trunc_page(sh[i].s_vaddr);
@@ -482,9 +498,9 @@ exec_pecoff_coff_prep_zmagic(struct imag
argp->a_ldbase = ldbase;
argp->a_ldexport = ldexport;
memcpy(argp->a_imghdr, wp->w_imghdr, sizeof(struct pecoff_imghdr) * 16);
- for (i = 0; i < 16; i++) {
+ for (i = 0; i < 16; i++)
argp->a_imghdr[i].i_vaddr += wp->w_base;
- }
+
imgp->proc->p_sysent = &pecoff_sysvec;
if (error)
goto fail;
@@ -510,7 +526,7 @@ exec_pecoff_coff_makecmds(struct image_p
struct coff_filehdr * fp, int peofs)
{
struct coff_aouthdr *ap;
- int error;
+ int error;
if (COFF_BADMAG(fp)) {
return ENOEXEC;
@@ -542,58 +558,58 @@ pecoff_signature(td, vp, dp)
int error;
char buf[512];
char *pesig;
- if (DOS_BADMAG(dp)) {
+
+ if (DOS_BADMAG(dp))
return ENOEXEC;
- }
+
error = pecoff_read_from(td, vp, dp->d_peofs, buf, sizeof(buf));
- if (error) {
+ if (error)
return error;
- }
+
pesig = buf;
- if (memcmp(pesig, signature, sizeof(signature) - 1) == 0) {
+ if (memcmp(pesig, signature, sizeof(signature) - 1) == 0)
return 0;
- }
+
return EFTYPE;
}
int
pecoff_read_from(td, vp, pos, buf, siz)
- struct thread *td;
- struct vnode *vp;
- int pos;
- caddr_t buf;
- int siz;
+ struct thread *td;
+ struct vnode *vp;
+ unsigned int pos;
+ caddr_t buf;
+ int siz;
{
- int error;
- size_t resid;
-
+ int error = 0;
+ size_t resid;
+
error = vn_rdwr(UIO_READ, vp, buf, siz, pos,
UIO_SYSSPACE, IO_NODELOCKED, td->td_ucred, NOCRED,
&resid, td);
- if (error)
+ if (error != 0)
return error;
if (resid != 0) {
return ENOEXEC;
}
- return 0;
+ return error;
}
static int
imgact_pecoff(struct image_params * imgp)
{
- const struct pecoff_dos_filehdr *dp = (const struct pecoff_dos_filehdr *)
- imgp->image_header;
+ const struct pecoff_dos_filehdr *dp;
struct coff_filehdr *fp;
- int error, peofs;
+ int error, peofs;
struct thread *td = curthread;
+ dp = (const struct pecoff_dos_filehdr *) imgp->image_header;
error = pecoff_signature(FIRST_THREAD_IN_PROC(imgp->proc),
imgp->vp, dp);
- if (error) {
+ if (error)
return -1;
- }
+
VOP_UNLOCK(imgp->vp, 0, td);
-
peofs = dp->d_peofs + sizeof(signature) - 1;
fp = malloc(PECOFF_HDR_SIZE, M_TEMP, M_WAITOK);
error = pecoff_read_from(FIRST_THREAD_IN_PROC(imgp->proc),
@@ -608,5 +624,8 @@ fail:
return error;
}
-static struct execsw pecoff_execsw = {imgact_pecoff, "FreeBSD PEcoff"};
+static struct execsw pecoff_execsw = {
+ imgact_pecoff,
+ "FreeBSD PEcoff"
+};
EXEC_SET(pecoff, pecoff_execsw);
diff -uprP /usr/src/sys/compat/pecoff/imgact_pecoff.h src/sys/compat/pecoff/imgact_pecoff.h
--- /usr/src/sys/compat/pecoff/imgact_pecoff.h Sat May 7 13:40:08 2005
+++ src/sys/compat/pecoff/imgact_pecoff.h Sat May 7 17:43:36 2005
@@ -17,6 +17,7 @@ struct pecoff_dos_filehdr {
#define PECOFF_DOS_HDR_SIZE (sizeof(struct pecoff_dos_filehdr))
#define DOS_BADMAG(dp) ((dp)->d_magic != PECOFF_DOS_MAGIC)
+#define PECOFF_PE_SIGNATURE "PE\0\0"
/*
* COFF file header
@@ -138,5 +139,11 @@ struct pecoff_args {
#define PECOFF_HDR_SIZE (COFF_HDR_SIZE + sizeof(struct pecoff_opthdr))
+#ifdef PECOFF_DEBUG
+#define PE_DEBUG(...) \
+ printf("PE: %s, %d: ", __func__, __LINE__); \
+ printf(__VA_ARGS__); \
+ printf("\n");
+#endif /* PECOFF_DEBUG */
-#endif
+#endif /* _PECOFF_EXEC_H_ */
diff -uprP /usr/src/sys/modules/pecoff/Makefile src/sys/modules/pecoff/Makefile
--- /usr/src/sys/modules/pecoff/Makefile Wed Sep 1 13:58:45 2004
+++ src/sys/modules/pecoff/Makefile Sat May 7 17:55:54 2005
@@ -7,6 +7,6 @@ MAINTAINER= takawata at FreeBSD.org
KMOD= pecoff
SRCS= imgact_pecoff.c opt_pecoff.h vnode_if.h
-CFLAGS+= -DDEBUG
+CFLAGS+= -DPECOFF_DEBUG
.include <bsd.kmod.mk>
--- diff.0.pecoff ends here ---
>Release-Note:
>Audit-Trail:
>Unformatted:
More information about the freebsd-bugs
mailing list