Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

[wliang]

[-- Attachment #1.1: Type: text/plain, Size: 920 bytes --]


Hi all,

I find a potential Use-after-free bug in QEMU 6.2.0, which is in handle_simd_shift_fpint_conv()(./target/arm/translate-a64.c).

At line 9048, a variable 'tcg_fpstatus' is freed by invoking tcg_temp_free_ptr(). However, at line 9050, the variable 'tcg_fpstatus' is subsequently use as the 3rd parameter of the function gen_helper_set_rmode. This may result in a use-after-free bug.


9048    tcg_temp_free_ptr(tcg_fpstatus);
9049    tcg_temp_free_i32(tcg_shift);
9050    gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);


I believe the bug can be fixed by invoking the gen_helper_set_rmode() before 'tcg_fpstatus' being freed by the tcg_temp_free_ptr().


 ---    tcg_temp_free_ptr(tcg_fpstatus);
9049    tcg_temp_free_i32(tcg_shift);
9050    gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
 +++    tcg_temp_free_ptr(tcg_fpstatus);
 
I'm looking forward to your confirmation.


Best,

Wentao

[-- Attachment #1.2: Type: text/html, Size: 3829 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: translate-a64.c.patch --]
[-- Type: text/x-patch; name=translate-a64.c.patch, Size: 400 bytes --]

--- ./target/arm/translate-a64.c	2022-02-23 15:06:32.212756633 +0800
+++ ./target/arm/translate-a64-PATCH.c	2022-02-23 21:13:15.604128138 +0800
@@ -9045,9 +9045,9 @@
         }
     }
 
-    tcg_temp_free_ptr(tcg_fpstatus);
     tcg_temp_free_i32(tcg_shift);
     gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
+    tcg_temp_free_ptr(tcg_fpstatus);
     tcg_temp_free_i32(tcg_rmode);
 }
 

Re: Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

[Richard Henderson]

On 2/23/22 04:33, wliang@stu.xidian.edu.cn wrote:
> 
> Hi all,
> 
> I find a potential Use-after-free bug in QEMU 6.2.0, which is in 
> handle_simd_shift_fpint_conv()(./target/arm/translate-a64.c).
> 
> At line 9048, a variable 'tcg_fpstatus' is freed by invoking tcg_temp_free_ptr(). However, 
> at line 9050, the variable 'tcg_fpstatus' is subsequently use as the 3rd parameter of the 
> function gen_helper_set_rmode. This may result in a use-after-free bug.
> 
> 
> 9048    tcg_temp_free_ptr(tcg_fpstatus);
> 9049    tcg_temp_free_i32(tcg_shift);
> 9050    gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
> 
> 
> I believe the bug can be fixed by invoking the gen_helper_set_rmode() before 
> 'tcg_fpstatus' being freed by the tcg_temp_free_ptr().
> 
> 
>   ---    tcg_temp_free_ptr(tcg_fpstatus);
> 9049    tcg_temp_free_i32(tcg_shift);
> 9050    gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
>   +++    tcg_temp_free_ptr(tcg_fpstatus);
> 
> I'm looking forward to your confirmation.

The fix is correct.  We just need the submission formatted properly, with your 
Signed-off-by tag.  When re-formatting, you can add my

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: Re: Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

[wliang]

[-- Attachment #1: Type: text/plain, Size: 320 bytes --]


> 
> The fix is correct.  We just need the submission formatted properly, with your 
> Signed-off-by tag.  When re-formatting, you can add my
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 

> r~

Hi guys,

Thank you for waiting for me.

Here is a new patch with Signed-off-by tags.

Best,
Wentao

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-a-potential-Use-after-free-bug-in-handle_simd_sh.patch --]
[-- Type: text/x-patch;  name=0001-Fix-a-potential-Use-after-free-bug-in-handle_simd_sh.patch, Size: 894 bytes --]

From 15129e2cec483a8416738b266bc3b36d56959f69 Mon Sep 17 00:00:00 2001
From: Wentao_Liang <Wentao_Liang_g@163.com>
Date: Fri, 25 Feb 2022 12:01:42 +0800
Subject: [PATCH] Fix a potential Use-after-free bug in
 handle_simd_shift_fpint_conv()

Signed-off-by: Wentao_Liang <Wentao_Liang_g@163.com>
---
 target/arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 5a1df25f91..d1a59fad9c 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -9045,9 +9045,9 @@ static void handle_simd_shift_fpint_conv(DisasContext *s, bool is_scalar,
         }
     }
 
-    tcg_temp_free_ptr(tcg_fpstatus);
     tcg_temp_free_i32(tcg_shift);
     gen_helper_set_rmode(tcg_rmode, tcg_rmode, tcg_fpstatus);
+    tcg_temp_free_ptr(tcg_fpstatus);
     tcg_temp_free_i32(tcg_rmode);
 }
 
-- 
2.25.1

Re: Re: Fix a potential Use-after-free bug in handle_simd_shift_fpint_conv() (v6.2.0).

[Peter Maydell]

On Fri, 25 Feb 2022 at 04:05, <wliang@stu.xidian.edu.cn> wrote:
>
>
> >
> > The fix is correct.  We just need the submission formatted properly, with your
> > Signed-off-by tag.  When re-formatting, you can add my
> >
> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> >
>
> > r~
>
> Hi guys,
>
> Thank you for waiting for me.
>
> Here is a new patch with Signed-off-by tags.

Thanks; I've applied this patch to target-arm.next (with some cleanup
of the commit message).

PS: the subject line suggests you're creating patches against the 6.2.0
release. For submitting patches to us, please always make them against
the current head-of-git, not against an old release version. (As it
happens, this patch is fine anyway, as the code in question hadn't
changed.)

thanks
-- PMM