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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> ---
>>>    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

Reply via email to