|
From: <sv...@va...> - 2016-10-01 11:54:55
|
Author: mjw
Date: Sat Oct 1 12:54:49 2016
New Revision: 15994
Log:
Fix crash in linux [rt_]sigaction wrapper with bad old/new sigaction handler.
Since we try to modify the old/new sigaction handler before passing it
to the kernel we must make sure that (if they aren't NULL) it is safe
to use. If not we should bail out early with EFAULT.
Bug #369362
Found by LTP testcases/kernel/syscalls/rt_sigaction/rt_sigaction02.
Modified:
trunk/NEWS
trunk/coregrind/m_syswrap/syswrap-linux.c
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Sat Oct 1 12:54:49 2016
@@ -184,6 +184,7 @@
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
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: trunk/coregrind/m_syswrap/syswrap-linux.c
==============================================================================
--- trunk/coregrind/m_syswrap/syswrap-linux.c (original)
+++ trunk/coregrind/m_syswrap/syswrap-linux.c Sat Oct 1 12:54:49 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)
{
|