1. Patchew
  2. QEMU
  3. [PATCH v7 0/8] Enable PC diversion via the plugin API
  4. View patch

[PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit

Florian Hofhammer posted 8 patches 4 weeks, 1 day ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Pierrick Bouvier <pierrick.bouvier@linaro.org>, Brian Cain <brian.cain@oss.qualcomm.com>, "Alex Bennée" <alex.bennee@linaro.org>, Alexandre Iooss <erdnaxe@crans.org>, Mahmoud Mandour <ma.mandourr@gmail.com>, John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Peter Maydell <peter.maydell@linaro.org>
[PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Florian Hofhammer 4 weeks, 1 day ago 
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Syscall arguments are abi_long in user code, and plugin syscall
interface works with uint64_t only.

According to C integer promotion rules, the value is sign extended
before becoming unsigned, thus setting high bits when only 32-bit lower
ones should have a significant value.

As a result, we need to clamp values we receive from user-code
accordingly.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 plugins/core.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/plugins/core.c b/plugins/core.c
index 42fd986593..d6173422e9 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
     }
 }
 
+static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
+                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
+                                    uint64_t *a7, uint64_t *a8)
+{
+    if (target_long_bits() == 32) {
+        const uint64_t mask = UINT32_MAX;
+        *a1 &= mask;
+        *a2 &= mask;
+        *a3 &= mask;
+        *a4 &= mask;
+        *a5 &= mask;
+        *a6 &= mask;
+        *a7 &= mask;
+        *a8 &= mask;
+    }
+}
+
 /*
  * Disable CFI checks.
  * The callback function has been loaded from an external library so we do not
@@ -531,6 +548,8 @@ qemu_plugin_vcpu_syscall(CPUState *cpu, int64_t num, uint64_t a1, uint64_t a2,
         return;
     }
 
+    clamp_syscall_arguments(&a1, &a2, &a3, &a4, &a5, &a6, &a7, &a8);
+
     QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {
         qemu_plugin_vcpu_syscall_cb_t func = cb->f.vcpu_syscall;
 
@@ -584,6 +603,8 @@ qemu_plugin_vcpu_syscall_filter(CPUState *cpu, int64_t num, uint64_t a1,
         return false;
     }
 
+    clamp_syscall_arguments(&a1, &a2, &a3, &a4, &a5, &a6, &a7, &a8);
+
     qemu_plugin_set_cb_flags(cpu, QEMU_PLUGIN_CB_RW_REGS);
 
     QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) {

-- 
2.53.0
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Philippe Mathieu-Daudé 4 weeks ago 
+Laurent

On 5/3/26 11:05, Florian Hofhammer wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Syscall arguments are abi_long in user code, and plugin syscall
> interface works with uint64_t only.
> 
> According to C integer promotion rules, the value is sign extended
> before becoming unsigned, thus setting high bits when only 32-bit lower
> ones should have a significant value.
> 
> As a result, we need to clamp values we receive from user-code
> accordingly.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   plugins/core.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/plugins/core.c b/plugins/core.c
> index 42fd986593..d6173422e9 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
>       }
>   }
>   
> +static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
> +                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
> +                                    uint64_t *a7, uint64_t *a8)
> +{
> +    if (target_long_bits() == 32) {

IIUC this is related to the target ABI, so maybe we want to
unconditionally use:

    const uint64_t mask = MAKE_64BIT_MASK(0, TARGET_ABI_BITS);

Maybe guarded with #if TARGET_ABI_BITS < 64?

> +        const uint64_t mask = UINT32_MAX;
> +        *a1 &= mask;
> +        *a2 &= mask;
> +        *a3 &= mask;
> +        *a4 &= mask;
> +        *a5 &= mask;
> +        *a6 &= mask;
> +        *a7 &= mask;
> +        *a8 &= mask;
> +    }
> +}
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Pierrick Bouvier 4 weeks ago 
On 3/5/26 9:33 AM, Philippe Mathieu-Daudé wrote:
> +Laurent
> 
> On 5/3/26 11:05, Florian Hofhammer wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Syscall arguments are abi_long in user code, and plugin syscall
>> interface works with uint64_t only.
>>
>> According to C integer promotion rules, the value is sign extended
>> before becoming unsigned, thus setting high bits when only 32-bit lower
>> ones should have a significant value.
>>
>> As a result, we need to clamp values we receive from user-code
>> accordingly.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    plugins/core.c | 21 +++++++++++++++++++++
>>    1 file changed, 21 insertions(+)
>>
>> diff --git a/plugins/core.c b/plugins/core.c
>> index 42fd986593..d6173422e9 100644
>> --- a/plugins/core.c
>> +++ b/plugins/core.c
>> @@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
>>        }
>>    }
>>    
>> +static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
>> +                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
>> +                                    uint64_t *a7, uint64_t *a8)
>> +{
>> +    if (target_long_bits() == 32) {
> 
> IIUC this is related to the target ABI, so maybe we want to
> unconditionally use:
> 
>      const uint64_t mask = MAKE_64BIT_MASK(0, TARGET_ABI_BITS);
> 
> Maybe guarded with #if TARGET_ABI_BITS < 64?
>

TARGET_ABI_BITS is a target specific define, but we are in common code 
here, so it's not usable.
As well, since this path is only exercised by linux-user (and not 
system), target_long_bits() should not be different from its runtime value.

>> +        const uint64_t mask = UINT32_MAX;
>> +        *a1 &= mask;
>> +        *a2 &= mask;
>> +        *a3 &= mask;
>> +        *a4 &= mask;
>> +        *a5 &= mask;
>> +        *a6 &= mask;
>> +        *a7 &= mask;
>> +        *a8 &= mask;
>> +    }
>> +}
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Pierrick Bouvier 4 weeks ago 
On 3/5/26 9:33 AM, Philippe Mathieu-Daudé wrote:
> +Laurent
> 
> On 5/3/26 11:05, Florian Hofhammer wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> Syscall arguments are abi_long in user code, and plugin syscall
>> interface works with uint64_t only.
>>
>> According to C integer promotion rules, the value is sign extended
>> before becoming unsigned, thus setting high bits when only 32-bit lower
>> ones should have a significant value.
>>
>> As a result, we need to clamp values we receive from user-code
>> accordingly.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    plugins/core.c | 21 +++++++++++++++++++++
>>    1 file changed, 21 insertions(+)
>>
>> diff --git a/plugins/core.c b/plugins/core.c
>> index 42fd986593..d6173422e9 100644
>> --- a/plugins/core.c
>> +++ b/plugins/core.c
>> @@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
>>        }
>>    }
>>    
>> +static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
>> +                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
>> +                                    uint64_t *a7, uint64_t *a8)
>> +{
>> +    if (target_long_bits() == 32) {
> 
> IIUC this is related to the target ABI, so maybe we want to
> unconditionally use:
> 
>      const uint64_t mask = MAKE_64BIT_MASK(0, TARGET_ABI_BITS);
>
> Maybe guarded with #if TARGET_ABI_BITS < 64?
>

More broadly, I really wonder why we use signed type for syscall 
arguments in linux-user, while I think they should always be unsigned.
But since soft freeze is coming very soon, it's not the time for a 
refactoring solution, thus this pragmatic, but correct, patch.

>> +        const uint64_t mask = UINT32_MAX;
>> +        *a1 &= mask;
>> +        *a2 &= mask;
>> +        *a3 &= mask;
>> +        *a4 &= mask;
>> +        *a5 &= mask;
>> +        *a6 &= mask;
>> +        *a7 &= mask;
>> +        *a8 &= mask;
>> +    }
>> +}
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Florian Hofhammer 4 weeks ago 
On 05/03/2026 18:38, Pierrick Bouvier wrote:
> On 3/5/26 9:33 AM, Philippe Mathieu-Daudé wrote:
>> +Laurent
>>
>> On 5/3/26 11:05, Florian Hofhammer wrote:
>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> Syscall arguments are abi_long in user code, and plugin syscall
>>> interface works with uint64_t only.
>>>
>>> According to C integer promotion rules, the value is sign extended
>>> before becoming unsigned, thus setting high bits when only 32-bit lower
>>> ones should have a significant value.
>>>
>>> As a result, we need to clamp values we receive from user-code
>>> accordingly.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    plugins/core.c | 21 +++++++++++++++++++++
>>>    1 file changed, 21 insertions(+)
>>>
>>> diff --git a/plugins/core.c b/plugins/core.c
>>> index 42fd986593..d6173422e9 100644
>>> --- a/plugins/core.c
>>> +++ b/plugins/core.c
>>> @@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
>>>        }
>>>    }
>>>    +static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
>>> +                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
>>> +                                    uint64_t *a7, uint64_t *a8)
>>> +{
>>> +    if (target_long_bits() == 32) {
>>
>> IIUC this is related to the target ABI, so maybe we want to
>> unconditionally use:
>>
>>      const uint64_t mask = MAKE_64BIT_MASK(0, TARGET_ABI_BITS);
>>
>> Maybe guarded with #if TARGET_ABI_BITS < 64?
>>
> 
> More broadly, I really wonder why we use signed type for syscall arguments in linux-user, while I think they should always be unsigned.
> But since soft freeze is coming very soon, it's not the time for a refactoring solution, thus this pragmatic, but correct, patch.

If you think it makes sense to refactor this for the next development
cycle/window, I'd happily give it a short and prepare an RFC patch for
it.

> 
>>> +        const uint64_t mask = UINT32_MAX;
>>> +        *a1 &= mask;
>>> +        *a2 &= mask;
>>> +        *a3 &= mask;
>>> +        *a4 &= mask;
>>> +        *a5 &= mask;
>>> +        *a6 &= mask;
>>> +        *a7 &= mask;
>>> +        *a8 &= mask;
>>> +    }
>>> +}
> 

Best regards,
Florian
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Pierrick Bouvier 3 weeks, 6 days ago 
On 3/5/26 11:46 PM, Florian Hofhammer wrote:
> On 05/03/2026 18:38, Pierrick Bouvier wrote:
>> On 3/5/26 9:33 AM, Philippe Mathieu-Daudé wrote:
>>> +Laurent
>>>
>>> On 5/3/26 11:05, Florian Hofhammer wrote:
>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>
>>>> Syscall arguments are abi_long in user code, and plugin syscall
>>>> interface works with uint64_t only.
>>>>
>>>> According to C integer promotion rules, the value is sign extended
>>>> before becoming unsigned, thus setting high bits when only 32-bit lower
>>>> ones should have a significant value.
>>>>
>>>> As a result, we need to clamp values we receive from user-code
>>>> accordingly.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>     plugins/core.c | 21 +++++++++++++++++++++
>>>>     1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/plugins/core.c b/plugins/core.c
>>>> index 42fd986593..d6173422e9 100644
>>>> --- a/plugins/core.c
>>>> +++ b/plugins/core.c
>>>> @@ -513,6 +513,23 @@ void qemu_plugin_tb_trans_cb(CPUState *cpu, struct qemu_plugin_tb *tb)
>>>>         }
>>>>     }
>>>>     +static void clamp_syscall_arguments(uint64_t *a1, uint64_t *a2, uint64_t *a3,
>>>> +                                    uint64_t *a4, uint64_t *a5, uint64_t *a6,
>>>> +                                    uint64_t *a7, uint64_t *a8)
>>>> +{
>>>> +    if (target_long_bits() == 32) {
>>>
>>> IIUC this is related to the target ABI, so maybe we want to
>>> unconditionally use:
>>>
>>>       const uint64_t mask = MAKE_64BIT_MASK(0, TARGET_ABI_BITS);
>>>
>>> Maybe guarded with #if TARGET_ABI_BITS < 64?
>>>
>>
>> More broadly, I really wonder why we use signed type for syscall arguments in linux-user, while I think they should always be unsigned.
>> But since soft freeze is coming very soon, it's not the time for a refactoring solution, thus this pragmatic, but correct, patch.
> 
> If you think it makes sense to refactor this for the next development
> cycle/window, I'd happily give it a short and prepare an RFC patch for
> it.
>

Sure, I would like to get feedback from Laurent and Richard first to see 
if we have a specific reason to have signed syscall arguments.

Regards,
Pierrick
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Alex Bennée 4 weeks ago 
Florian Hofhammer <florian.hofhammer@epfl.ch> writes:

> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>
> Syscall arguments are abi_long in user code, and plugin syscall
> interface works with uint64_t only.
>
> According to C integer promotion rules, the value is sign extended
> before becoming unsigned, thus setting high bits when only 32-bit lower
> ones should have a significant value.
>
> As a result, we need to clamp values we receive from user-code
> accordingly.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v7 1/8] plugins/core: clamp syscall arguments if target is 32-bit
Posted by Pierrick Bouvier 4 weeks ago 
On 3/5/26 2:05 AM, Florian Hofhammer wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> Syscall arguments are abi_long in user code, and plugin syscall
> interface works with uint64_t only.
> 
> According to C integer promotion rules, the value is sign extended
> before becoming unsigned, thus setting high bits when only 32-bit lower
> ones should have a significant value.
> 
> As a result, we need to clamp values we receive from user-code
> accordingly.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   plugins/core.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 

@Alex, Philippe: may I ask you a review on this one?

Regards,
Pierrick

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.