[PATCH v2] accel/tcg: Ensure get_page_addr_code_hostp nulls output on failure

Panda Jiang posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260128074236.49624-1-3160104094@zju.edu.cn
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>
accel/tcg/cputlb.c | 4 ++++
1 file changed, 4 insertions(+)
[PATCH v2] accel/tcg: Ensure get_page_addr_code_hostp nulls output on failure
Posted by Panda Jiang 1 week, 2 days ago
In tb_gen_code(), get_page_addr_code_hostp() is called to get a host
mapping for a guest PC. If the function fails (e.g., for an I/O region)
and returns -1, it previously left its output pointer parameter
('host_pc' in the caller) unmodified.

If the caller's variable was uninitialized, this leads to undefined
behavior when it is later used, for example in setjmp_gen_code(). This
was observed as a segmentation fault when running QEMU with the
'-d in_asm' logging option when mmu translation fails.

As suggested by Richard Henderson, fix this within get_page_addr_code_hostp()
itself rather than in the caller. Ensure that in all failure paths where -1
is returned, the output pointer is explicitly set to NULL.

Signed-off-by: Panda Jiang <3160104094@zju.edu.cn>
---
+ Changes in v2:
+ - Moved the fix from the caller (tb_gen_code) to the callee
+   (get_page_addr_code_hostp).
+ - Set the output pointer to NULL on failure paths inside
+   get_page_addr_code_hostp, as suggested.
+ - Updated commit message to reflect the new approach.
+
 accel/tcg/cputlb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 6900a12682..6d7cfd2b5a 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1543,6 +1543,10 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
     CPUTLBEntryFull *full;
     void *p;
 
+    if (hostp) {
+        *hostp = NULL;
+    }
+
     (void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
                                 cpu_mmu_index(env_cpu(env), true), false,
                                 &p, &full, 0, false);
-- 
2.39.2 (Apple Git-143)
Re: [PATCH v2] accel/tcg: Ensure get_page_addr_code_hostp nulls output on failure
Posted by Philippe Mathieu-Daudé 1 week, 2 days ago
On Wed, 28 Jan 2026 at 08:43, Panda Jiang <3160104094@zju.edu.cn> wrote:
>
> In tb_gen_code(), get_page_addr_code_hostp() is called to get a host
> mapping for a guest PC. If the function fails (e.g., for an I/O region)
> and returns -1, it previously left its output pointer parameter
> ('host_pc' in the caller) unmodified.
>
> If the caller's variable was uninitialized, this leads to undefined
> behavior when it is later used, for example in setjmp_gen_code(). This
> was observed as a segmentation fault when running QEMU with the
> '-d in_asm' logging option when mmu translation fails.
>
> As suggested by Richard Henderson, fix this within get_page_addr_code_hostp()
> itself rather than in the caller. Ensure that in all failure paths where -1
> is returned, the output pointer is explicitly set to NULL.
>
> Signed-off-by: Panda Jiang <3160104094@zju.edu.cn>
> ---
> + Changes in v2:
> + - Moved the fix from the caller (tb_gen_code) to the callee
> +   (get_page_addr_code_hostp).
> + - Set the output pointer to NULL on failure paths inside
> +   get_page_addr_code_hostp, as suggested.
> + - Updated commit message to reflect the new approach.

See Richard's elsewhere fix:
https://lore.kernel.org/qemu-devel/20260128010715.347776-3-richard.henderson@linaro.org/

>  accel/tcg/cputlb.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 6900a12682..6d7cfd2b5a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1543,6 +1543,10 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
>      CPUTLBEntryFull *full;
>      void *p;
>
> +    if (hostp) {
> +        *hostp = NULL;
> +    }
> +
>      (void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
>                                  cpu_mmu_index(env_cpu(env), true), false,
>                                  &p, &full, 0, false);
> --
> 2.39.2 (Apple Git-143)
>
>
Re: [PATCH v2] accel/tcg: Ensure get_page_addr_code_hostp nulls output on failure
Posted by Panda Jiang 1 week, 1 day ago
On 1/29/26 1:03 AM, Philippe Mathieu-Daudé wrote:
> On Wed, 28 Jan 2026 at 08:43, Panda Jiang <3160104094@zju.edu.cn> wrote:
>>
>> In tb_gen_code(), get_page_addr_code_hostp() is called to get a host
>> mapping for a guest PC. If the function fails (e.g., for an I/O region)
>> and returns -1, it previously left its output pointer parameter
>> ('host_pc' in the caller) unmodified.
>>
>> If the caller's variable was uninitialized, this leads to undefined
>> behavior when it is later used, for example in setjmp_gen_code(). This
>> was observed as a segmentation fault when running QEMU with the
>> '-d in_asm' logging option when mmu translation fails.
>>
>> As suggested by Richard Henderson, fix this within get_page_addr_code_hostp()
>> itself rather than in the caller. Ensure that in all failure paths where -1
>> is returned, the output pointer is explicitly set to NULL.
>>
>> Signed-off-by: Panda Jiang <3160104094@zju.edu.cn>
>> ---
>> + Changes in v2:
>> + - Moved the fix from the caller (tb_gen_code) to the callee
>> +   (get_page_addr_code_hostp).
>> + - Set the output pointer to NULL on failure paths inside
>> +   get_page_addr_code_hostp, as suggested.
>> + - Updated commit message to reflect the new approach.
> 
> See Richard's elsewhere fix:
> https://lore.kernel.org/qemu-devel/20260128010715.347776-3-richard.henderson@linaro.org/
> 
>>   accel/tcg/cputlb.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
>> index 6900a12682..6d7cfd2b5a 100644
>> --- a/accel/tcg/cputlb.c
>> +++ b/accel/tcg/cputlb.c
>> @@ -1543,6 +1543,10 @@ tb_page_addr_t get_page_addr_code_hostp(CPUArchState *env, vaddr addr,
>>       CPUTLBEntryFull *full;
>>       void *p;
>>
>> +    if (hostp) {
>> +        *hostp = NULL;
>> +    }
>> +
>>       (void)probe_access_internal(env_cpu(env), addr, 1, MMU_INST_FETCH,
>>                                   cpu_mmu_index(env_cpu(env), true), false,
>>                                   &p, &full, 0, false);
>> --
>> 2.39.2 (Apple Git-143)
>>
>>

Hi Philippe,

Got it. Thank you for pointing this out and providing the link. I wasn't 
aware that this was already fixed.

Apologies for the noise.

Thanks again,
Panda Jiang