Patchwork [for-1.6,2/2] linux-user/signal.c: Avoid using uninitialized data in ARM sigreturn

login
register
mail settings
Submitter Peter Maydell
Date July 29, 2013, 11 a.m.
Message ID <1375095632-13735-3-git-send-email-peter.maydell@linaro.org>
Download mbox | patch
Permalink /patch/262717/
State New
Headers show

Comments

Peter Maydell - July 29, 2013, 11 a.m.
Rephrase code used in ARM sigreturn functions to avoid using
uninitialized variables. This fixes one genuine problem ('frame'
would not be initialized if we took the error-exit path because
our stackpointer was misaligned) and one which is clang being
alarmist (frame_addr wouldn't be initialized, though this is
harmless since unlock_user_struct ignores its second argument
in these cases; however since we don't generally make use of
this not-really-documented effect it's better avoided).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/signal.c |   36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Patch

diff --git a/linux-user/signal.c b/linux-user/signal.c
index d63777d..23d65da 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1552,7 +1552,7 @@  restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
 static long do_sigreturn_v1(CPUARMState *env)
 {
         abi_ulong frame_addr;
-	struct sigframe_v1 *frame;
+        struct sigframe_v1 *frame = NULL;
 	target_sigset_t set;
         sigset_t host_set;
         int i;
@@ -1562,10 +1562,11 @@  static long do_sigreturn_v1(CPUARMState *env)
 	 * then 'sp' should be word aligned here.  If it's
 	 * not, then the user is trying to mess with us.
 	 */
-	if (env->regs[13] & 7)
-		goto badframe;
-
         frame_addr = env->regs[13];
+        if (frame_addr & 7) {
+            goto badframe;
+        }
+
 	if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
                 goto badframe;
 
@@ -1693,17 +1694,18 @@  static int do_sigframe_return_v2(CPUARMState *env, target_ulong frame_addr,
 static long do_sigreturn_v2(CPUARMState *env)
 {
         abi_ulong frame_addr;
-	struct sigframe_v2 *frame;
+        struct sigframe_v2 *frame = NULL;
 
 	/*
 	 * Since we stacked the signal on a 64-bit boundary,
 	 * then 'sp' should be word aligned here.  If it's
 	 * not, then the user is trying to mess with us.
 	 */
-	if (env->regs[13] & 7)
-		goto badframe;
-
         frame_addr = env->regs[13];
+        if (frame_addr & 7) {
+            goto badframe;
+        }
+
 	if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
                 goto badframe;
 
@@ -1731,7 +1733,7 @@  long do_sigreturn(CPUARMState *env)
 static long do_rt_sigreturn_v1(CPUARMState *env)
 {
         abi_ulong frame_addr;
-	struct rt_sigframe_v1 *frame;
+        struct rt_sigframe_v1 *frame = NULL;
         sigset_t host_set;
 
 	/*
@@ -1739,10 +1741,11 @@  static long do_rt_sigreturn_v1(CPUARMState *env)
 	 * then 'sp' should be word aligned here.  If it's
 	 * not, then the user is trying to mess with us.
 	 */
-	if (env->regs[13] & 7)
-		goto badframe;
-
         frame_addr = env->regs[13];
+        if (frame_addr & 7) {
+            goto badframe;
+        }
+
 	if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
                 goto badframe;
 
@@ -1772,17 +1775,18 @@  badframe:
 static long do_rt_sigreturn_v2(CPUARMState *env)
 {
         abi_ulong frame_addr;
-	struct rt_sigframe_v2 *frame;
+        struct rt_sigframe_v2 *frame = NULL;
 
 	/*
 	 * Since we stacked the signal on a 64-bit boundary,
 	 * then 'sp' should be word aligned here.  If it's
 	 * not, then the user is trying to mess with us.
 	 */
-	if (env->regs[13] & 7)
-		goto badframe;
-
         frame_addr = env->regs[13];
+        if (frame_addr & 7) {
+            goto badframe;
+        }
+
 	if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1))
                 goto badframe;