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

[Peter Maydell]

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 <address@hidden>
---
 linux-user/signal.c |   36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

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;
 
-- 
1.7.9.5