From: Mark Wielaard Date: Sat, 1 Oct 2016 11:54:49 +0000 (+0000) Subject: Fix crash in linux [rt_]sigaction wrapper with bad old/new sigaction handler. X-Git-Tag: svn/VALGRIND_3_13_0~372 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e1e9a775649a068c2983b91eb324d078abf969b3;p=thirdparty%2Fvalgrind.git 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. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15994 --- diff --git a/NEWS b/NEWS index 051c1a6a4f..c79e9f36dd 100644 --- a/NEWS +++ b/NEWS @@ -184,6 +184,7 @@ where XXXXXX is the bug number as listed below. 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 diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c index 6f9a8beedd..e53de6c985 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -3277,7 +3277,7 @@ PRE(sys_sigaction) 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 @@ PRE(sys_sigaction) 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(sys_rt_sigaction) 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 + /* 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 { - SET_STATUS_from_SysRes( - VG_(do_sys_sigaction)(ARG1, (const vki_sigaction_toK_t *)ARG2, - (vki_sigaction_fromK_t *)ARG3) - ); + // 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) {