Menu
▾
▴
[Valgrind-developers] Valgrind: r16015 - in /branches/VALGRIND_3_12_BRANCH: ./ NEWS coregrind/m_syswrap/syswrap-generic.c coregrind/m_syswrap/syswrap-linux.c coregrind/m_syswrap/syswrap-x86-linux.c
Author: sewardj
Date: Wed Oct 5 06:42:01 2016
New Revision: 16015
Log:
Merge from trunk:
r15990 Fix pre_mem_read_sockaddr crash on invalid syscall arguments.
Bug #369356.
r15991 Fix crash in msghdr_foreachfield when iov_len isn't safe to dereference.
Bug #369359
r15992 Fix crash when old/new sigprocmask isn't safe to dereference.
Bug #369360.
r15993 Fix crash in vmsplice linux kernel wrapper when iovec is bad.
Bug #369361.
r15994 Fix crash in linux [rt_]sigaction wrapper with bad old/new
sigaction handler. Bug #369362
r15995 Fix crash in sys_modify_ldt wrapper on bad ptr.
Bug #369383.
r15996 linux-x86 check get/set_thread_area pointer before use.
Bug #369402.
r15997 Don't check bad iovec array in process_vm_readv/writev.
Bug #369441.
r15998 Don't crash, but warn and return EINVAL on unknown fcntl command.
Modified:
branches/VALGRIND_3_12_BRANCH/ (props changed)
branches/VALGRIND_3_12_BRANCH/NEWS (contents, props changed)
branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-generic.c
branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-linux.c
branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-x86-linux.c
Modified: branches/VALGRIND_3_12_BRANCH/NEWS
==============================================================================
--- branches/VALGRIND_3_12_BRANCH/NEWS (original)
+++ branches/VALGRIND_3_12_BRANCH/NEWS Wed Oct 5 06:42:01 2016
@@ -179,6 +179,15 @@
361253 [s390x] ex_clone.c:42: undefined reference to `pthread_create'
369169 ppc64 fails jm_int_isa_2_07 test
369209 valgrind loops and eats up all memory if cwd doesn't exist.
+369356 pre_mem_read_sockaddr syscall wrapper can crash with bad sockaddr
+369359 msghdr_foreachfield can crash when handling bad iovec
+369360 Bad sigprocmask old or new sets can crash valgrind
+369361 vmsplice syscall wrapper crashes on bad iovec
+369362 Bad sigaction arguments crash valgrind
+369383 x86 sys_modify_ldt wrapper crashes on bad ptr
+369402 Bad set/get_thread_area pointer crashes valgrind
+369441 bad lvec argument crashes process_vm_readv/writev syscall wrappers
+369446 valgrind crashes on unknown fcntl command
n-i-bz Fix incorrect (or infinite loop) unwind on RHEL7 x86 and amd64
n-i-bz massif --pages-as-heap=yes does not report peak caused by mmap+munmap
Modified: branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-generic.c
==============================================================================
--- branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-generic.c (original)
+++ branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-generic.c Wed Oct 5 06:42:01 2016
@@ -1056,15 +1056,19 @@
VG_(sprintf) ( fieldName, "(%s.msg_iov)", name );
- foreach_func ( tid, True, fieldName,
- (Addr)iov, msg->msg_iovlen * sizeof( struct vki_iovec ) );
-
- for ( i = 0; i < msg->msg_iovlen; ++i, ++iov ) {
- UInt iov_len = iov->iov_len <= length ? iov->iov_len : length;
- VG_(sprintf) ( fieldName, "(%s.msg_iov[%u])", name, i );
- foreach_func ( tid, False, fieldName,
- (Addr)iov->iov_base, iov_len );
- length = length - iov_len;
+ if (ML_(safe_to_deref)(&msg->msg_iovlen, sizeof (UInt))) {
+ foreach_func ( tid, True, fieldName, (Addr)iov,
+ msg->msg_iovlen * sizeof( struct vki_iovec ) );
+
+ for ( i = 0; i < msg->msg_iovlen && length > 0; ++i, ++iov ) {
+ if (ML_(safe_to_deref)(&iov->iov_len, sizeof (UInt))) {
+ UInt iov_len = iov->iov_len <= length ? iov->iov_len : length;
+ VG_(sprintf) ( fieldName, "(%s.msg_iov[%u])", name, i );
+ foreach_func ( tid, False, fieldName,
+ (Addr)iov->iov_base, iov_len );
+ length = length - iov_len;
+ }
+ }
}
}
@@ -1128,12 +1132,20 @@
VG_(sprintf) ( outmsg, description, "sa_family" );
PRE_MEM_READ( outmsg, (Addr) &sa->sa_family, sizeof(vki_sa_family_t));
+ /* Don't do any extra checking if we cannot determine the sa_family. */
+ if (! ML_(safe_to_deref) (&sa->sa_family, sizeof(vki_sa_family_t))) {
+ VG_(free) (outmsg);
+ return;
+ }
+
switch (sa->sa_family) {
case VKI_AF_UNIX:
- VG_(sprintf) ( outmsg, description, "sun_path" );
- PRE_MEM_RASCIIZ( outmsg, (Addr) saun->sun_path );
- // GrP fixme max of sun_len-2? what about nul char?
+ if (ML_(safe_to_deref) (&saun->sun_path, sizeof (Addr))) {
+ VG_(sprintf) ( outmsg, description, "sun_path" );
+ PRE_MEM_RASCIIZ( outmsg, (Addr) saun->sun_path );
+ // GrP fixme max of sun_len-2? what about nul char?
+ }
break;
case VKI_AF_INET:
Modified: branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-linux.c
==============================================================================
--- branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-linux.c (original)
+++ branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-linux.c Wed Oct 5 06:42:01 2016
@@ -3277,7 +3277,7 @@
PRE_MEM_READ( "sigaction(act->sa_handler)", (Addr)&sa->ksa_handler, sizeof(sa->ksa_handler));
PRE_MEM_READ( "sigaction(act->sa_mask)", (Addr)&sa->sa_mask, sizeof(sa->sa_mask));
PRE_MEM_READ( "sigaction(act->sa_flags)", (Addr)&sa->sa_flags, sizeof(sa->sa_flags));
- if (ML_(safe_to_deref)(sa,sizeof(sa))
+ if (ML_(safe_to_deref)(sa,sizeof(sa))
&& (sa->sa_flags & VKI_SA_RESTORER))
PRE_MEM_READ( "sigaction(act->sa_restorer)", (Addr)&sa->sa_restorer, sizeof(sa->sa_restorer));
}
@@ -3287,26 +3287,43 @@
oldp = &old;
}
- if (ARG2 != 0) {
- struct vki_old_sigaction *oldnew = (struct vki_old_sigaction *)ARG2;
+ /* If the new or old sigaction is not NULL, but the structs
+ aren't accessible then sigaction returns EFAULT and we cannot
+ use either struct for our own bookkeeping. Just fail early. */
+ if (ARG2 != 0
+ && ! ML_(safe_to_deref)((void *)ARG2,
+ sizeof(struct vki_old_sigaction))) {
+ VG_(umsg)("Warning: bad act handler address %p in sigaction()\n",
+ (void *)ARG2);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ } else if ((ARG3 != 0
+ && ! ML_(safe_to_deref)((void *)ARG3,
+ sizeof(struct vki_old_sigaction)))) {
+ VG_(umsg)("Warning: bad oldact handler address %p in sigaction()\n",
+ (void *)ARG3);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ } else {
+ if (ARG2 != 0) {
+ struct vki_old_sigaction *oldnew = (struct vki_old_sigaction *)ARG2;
- new.ksa_handler = oldnew->ksa_handler;
- new.sa_flags = oldnew->sa_flags;
- new.sa_restorer = oldnew->sa_restorer;
- convert_sigset_to_rt(&oldnew->sa_mask, &new.sa_mask);
- newp = &new;
- }
+ new.ksa_handler = oldnew->ksa_handler;
+ new.sa_flags = oldnew->sa_flags;
+ new.sa_restorer = oldnew->sa_restorer;
+ convert_sigset_to_rt(&oldnew->sa_mask, &new.sa_mask);
+ newp = &new;
+ }
- SET_STATUS_from_SysRes( VG_(do_sys_sigaction)(ARG1, newp, oldp) );
+ SET_STATUS_from_SysRes( VG_(do_sys_sigaction)(ARG1, newp, oldp) );
- if (ARG3 != 0 && SUCCESS && RES == 0) {
- struct vki_old_sigaction *oldold = (struct vki_old_sigaction *)ARG3;
+ if (ARG3 != 0 && SUCCESS && RES == 0) {
+ struct vki_old_sigaction *oldold = (struct vki_old_sigaction *)ARG3;
- oldold->ksa_handler = oldp->ksa_handler;
- oldold->sa_flags = oldp->sa_flags;
- oldold->sa_restorer = oldp->sa_restorer;
- oldold->sa_mask = oldp->sa_mask.sig[0];
- }
+ oldold->ksa_handler = oldp->ksa_handler;
+ oldold->sa_flags = oldp->sa_flags;
+ oldold->sa_restorer = oldp->sa_restorer;
+ oldold->sa_mask = oldp->sa_mask.sig[0];
+ }
+ }
}
POST(sys_sigaction)
{
@@ -3373,20 +3390,39 @@
PRE_MEM_READ( "rt_sigaction(act->sa_handler)", (Addr)&sa->ksa_handler, sizeof(sa->ksa_handler));
PRE_MEM_READ( "rt_sigaction(act->sa_mask)", (Addr)&sa->sa_mask, sizeof(sa->sa_mask));
PRE_MEM_READ( "rt_sigaction(act->sa_flags)", (Addr)&sa->sa_flags, sizeof(sa->sa_flags));
- if (sa->sa_flags & VKI_SA_RESTORER)
+ if (ML_(safe_to_deref)(sa,sizeof(sa))
+ && (sa->sa_flags & VKI_SA_RESTORER))
PRE_MEM_READ( "rt_sigaction(act->sa_restorer)", (Addr)&sa->sa_restorer, sizeof(sa->sa_restorer));
}
if (ARG3 != 0)
PRE_MEM_WRITE( "rt_sigaction(oldact)", ARG3, sizeof(vki_sigaction_fromK_t));
- // XXX: doesn't seem right to be calling do_sys_sigaction for
- // sys_rt_sigaction... perhaps this function should be renamed
- // VG_(do_sys_rt_sigaction)() --njn
-
- SET_STATUS_from_SysRes(
- VG_(do_sys_sigaction)(ARG1, (const vki_sigaction_toK_t *)ARG2,
- (vki_sigaction_fromK_t *)ARG3)
- );
+ /* If the new or old sigaction is not NULL, but the structs
+ aren't accessible then sigaction returns EFAULT and we cannot
+ use either struct for our own bookkeeping. Just fail early. */
+ if (ARG2 != 0
+ && ! ML_(safe_to_deref)((void *)ARG2,
+ sizeof(vki_sigaction_toK_t))) {
+ VG_(umsg)("Warning: bad act handler address %p in rt_sigaction()\n",
+ (void *)ARG2);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ } else if ((ARG3 != 0
+ && ! ML_(safe_to_deref)((void *)ARG3,
+ sizeof(vki_sigaction_fromK_t)))) {
+ VG_(umsg)("Warning: bad oldact handler address %p in rt_sigaction()\n",
+ (void *)ARG3);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ } else {
+
+ // XXX: doesn't seem right to be calling do_sys_sigaction for
+ // sys_rt_sigaction... perhaps this function should be renamed
+ // VG_(do_sys_rt_sigaction)() --njn
+
+ SET_STATUS_from_SysRes(
+ VG_(do_sys_sigaction)(ARG1, (const vki_sigaction_toK_t *)ARG2,
+ (vki_sigaction_fromK_t *)ARG3)
+ );
+ }
}
POST(sys_rt_sigaction)
{
@@ -3408,8 +3444,23 @@
PRE_MEM_WRITE( "rt_sigprocmask(oldset)", ARG3, sizeof(vki_sigset_t));
// Like the kernel, we fail if the sigsetsize is not exactly what we expect.
+ // Since we want to use the set and oldset for bookkeeping we also want
+ // to make sure they are addressable otherwise, like the kernel, we EFAULT.
if (sizeof(vki_sigset_t) != ARG4)
- SET_STATUS_Failure( VKI_EMFILE );
+ SET_STATUS_Failure( VKI_EINVAL );
+ else if (ARG2 != 0
+ && ! ML_(safe_to_deref)((void *)ARG2, sizeof(vki_sigset_t))) {
+ VG_(dmsg)("Warning: Bad set handler address %p in sigprocmask\n",
+ (void *)ARG2);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ }
+ else if (ARG3 != 0
+ && ! ML_(safe_to_deref)((void *)ARG3, sizeof(vki_sigset_t))) {
+ VG_(dmsg)("Warning: Bad oldset address %p in sigprocmask\n",
+ (void *)ARG3);
+ SET_STATUS_Failure ( VKI_EFAULT );
+ }
+
else {
SET_STATUS_from_SysRes(
VG_(do_sys_sigprocmask) ( tid, ARG1 /*how*/,
@@ -4953,8 +5004,8 @@
ARG2, ARG3 * sizeof(struct vki_iovec) );
PRE_MEM_READ( "process_vm_readv(rvec)",
ARG4, ARG5 * sizeof(struct vki_iovec) );
- if (ARG2 != 0) {
- /* TODO: Don't do any of the following if lvec is invalid */
+ if (ARG2 != 0
+ && ML_(safe_to_deref) ((void *)ARG2, sizeof(struct vki_iovec) * ARG3)) {
const struct vki_iovec *vec = (const struct vki_iovec *)ARG2;
UInt i;
for (i = 0; i < ARG3; i++)
@@ -4991,8 +5042,8 @@
ARG2, ARG3 * sizeof(struct vki_iovec) );
PRE_MEM_READ( "process_vm_writev(rvec)",
ARG4, ARG5 * sizeof(struct vki_iovec) );
- if (ARG2 != 0) {
- /* TODO: Don't do any of the following if lvec is invalid */
+ if (ARG2 != 0
+ && ML_(safe_to_deref) ((void *)ARG2, sizeof(struct vki_iovec) * ARG3)) {
const struct vki_iovec *vec = (const struct vki_iovec *)ARG2;
UInt i;
for (i = 0; i < ARG3; i++)
@@ -5295,10 +5346,14 @@
for (iov = (struct vki_iovec *)ARG2;
iov < (struct vki_iovec *)ARG2 + ARG3; iov++)
{
- if ((fdfl & VKI_O_ACCMODE) == VKI_O_RDONLY)
- PRE_MEM_WRITE( "vmsplice(iov[...])", (Addr)iov->iov_base, iov->iov_len );
- else
- PRE_MEM_READ( "vmsplice(iov[...])", (Addr)iov->iov_base, iov->iov_len );
+ if (ML_(safe_to_deref) (iov, sizeof(struct vki_iovec))) {
+ if ((fdfl & VKI_O_ACCMODE) == VKI_O_RDONLY)
+ PRE_MEM_WRITE( "vmsplice(iov[...])",
+ (Addr)iov->iov_base, iov->iov_len );
+ else
+ PRE_MEM_READ( "vmsplice(iov[...])",
+ (Addr)iov->iov_base, iov->iov_len );
+ }
}
}
}
@@ -5432,7 +5487,8 @@
default:
PRINT("sys_fcntl[UNKNOWN] ( %lu, %lu, %lu )", ARG1, ARG2, ARG3);
- I_die_here;
+ VG_(umsg)("Warning: unimplemented fcntl command: %lu\n", ARG2);
+ SET_STATUS_Failure( VKI_EINVAL );
break;
}
Modified: branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-x86-linux.c
==============================================================================
--- branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-x86-linux.c (original)
+++ branches/VALGRIND_3_12_BRANCH/coregrind/m_syswrap/syswrap-x86-linux.c Wed Oct 5 06:42:01 2016
@@ -596,24 +596,31 @@
static SysRes sys_modify_ldt ( ThreadId tid,
Int func, void* ptr, UInt bytecount )
{
- SysRes ret = VG_(mk_SysRes_Error)( VKI_ENOSYS );
+ SysRes ret;
- switch (func) {
- case 0:
- ret = read_ldt(tid, ptr, bytecount);
- break;
- case 1:
- ret = write_ldt(tid, ptr, bytecount, 1);
- break;
- case 2:
- VG_(unimplemented)("sys_modify_ldt: func == 2");
- /* god knows what this is about */
- /* ret = read_default_ldt(ptr, bytecount); */
- /*UNREACHED*/
- break;
- case 0x11:
- ret = write_ldt(tid, ptr, bytecount, 0);
- break;
+ if (func != 0 && func != 1 && func != 2 && func != 0x11) {
+ ret = VG_(mk_SysRes_Error)( VKI_ENOSYS );
+ } else if (ptr != NULL && ! ML_(safe_to_deref)(ptr, bytecount)) {
+ ret = VG_(mk_SysRes_Error)( VKI_EFAULT );
+ } else {
+ switch (func) {
+ case 0:
+ ret = read_ldt(tid, ptr, bytecount);
+ break;
+ case 1:
+ ret = write_ldt(tid, ptr, bytecount, 1);
+ break;
+ case 2:
+ ret = VG_(mk_SysRes_Error)( VKI_ENOSYS );
+ VG_(unimplemented)("sys_modify_ldt: func == 2");
+ /* god knows what this is about */
+ /* ret = read_default_ldt(ptr, bytecount); */
+ /*UNREACHED*/
+ break;
+ case 0x11:
+ ret = write_ldt(tid, ptr, bytecount, 0);
+ break;
+ }
}
return ret;
}
@@ -627,7 +634,7 @@
vg_assert(8 == sizeof(VexGuestX86SegDescr));
vg_assert(sizeof(HWord) == sizeof(VexGuestX86SegDescr*));
- if (info == NULL)
+ if (info == NULL || ! ML_(safe_to_deref)(info, sizeof(vki_modify_ldt_t)))
return VG_(mk_SysRes_Error)( VKI_EFAULT );
gdt = (VexGuestX86SegDescr*)VG_(threads)[tid].arch.vex.guest_GDT;
@@ -679,7 +686,7 @@
vg_assert(sizeof(HWord) == sizeof(VexGuestX86SegDescr*));
vg_assert(8 == sizeof(VexGuestX86SegDescr));
- if (info == NULL)
+ if (info == NULL || ! ML_(safe_to_deref)(info, sizeof(vki_modify_ldt_t)))
return VG_(mk_SysRes_Error)( VKI_EFAULT );
idx = info->entry_number;
|
