Xen devel - Aug 2012 - Multicall result missing sign extension in Xen or Linux

While trying to figure out why a failing component of a multicall did not
properly return its result, I discovered that multicall results are not
sign-extended when placed in the unsigned long result field. For hypercalls
such as do_mmu_update which return a (signed) int, this results in Linux
incorrectly thinking the hypercall succeeded when it has actually failed
since arch/x86/xen/multicalls.c uses a signed long for "result" and
checks
(b->entries[i].result < 0).

Is this a bug in Xen (using the wrong return type for do_mmu_op and other
hypercalls) or in Linux (assuming all returns are signed longs)? One or the
other needs to be changed, because the current setup is silently hiding
failed memory mapping operations.

-- 
Daniel De Graaf
National Security Agency

On 08/03/2012 11:57 AM, Daniel De Graaf wrote:
> While trying to figure out why a failing component of a multicall did not
> properly return its result, I discovered that multicall results are not
> sign-extended when placed in the unsigned long result field. For hypercalls
> such as do_mmu_update which return a (signed) int, this results in Linux
> incorrectly thinking the hypercall succeeded when it has actually failed
> since arch/x86/xen/multicalls.c uses a signed long for "result"
and checks
> (b->entries[i].result < 0).
>
> Is this a bug in Xen (using the wrong return type for do_mmu_op and other
> hypercalls) or in Linux (assuming all returns are signed longs)? One or the
> other needs to be changed, because the current setup is silently hiding
> failed memory mapping operations.
>
Ah, that explains a long-standing mystery for me that I never got around
to investigating.

If Xen is populating a 64-bit result with non-signed-extended 32-bit
result, then it sounds like a Xen bug, but one that''s effectively baked
into the ABI now.  Is there any risk of the unextended 32bit error being
ambiguously confused with a legitimate result?

    J

On 08/03/2012 03:00 PM, Jeremy Fitzhardinge wrote:
> On 08/03/2012 11:57 AM, Daniel De Graaf wrote:
>> While trying to figure out why a failing component of a multicall did
not
>> properly return its result, I discovered that multicall results are not
>> sign-extended when placed in the unsigned long result field. For
hypercalls
>> such as do_mmu_update which return a (signed) int, this results in
Linux
>> incorrectly thinking the hypercall succeeded when it has actually
failed
>> since arch/x86/xen/multicalls.c uses a signed long for
"result" and checks
>> (b->entries[i].result < 0).
>>
>> Is this a bug in Xen (using the wrong return type for do_mmu_op and
other
>> hypercalls) or in Linux (assuming all returns are signed longs)? One or
the
>> other needs to be changed, because the current setup is silently hiding
>> failed memory mapping operations.
>>
> 
> Ah, that explains a long-standing mystery for me that I never got around
> to investigating.
> 
> If Xen is populating a 64-bit result with non-signed-extended 32-bit
> result, then it sounds like a Xen bug, but one that''s effectively
baked
> into the ABI now.  Is there any risk of the unextended 32bit error being
> ambiguously confused with a legitimate result?
> 
>     J
> 
> 
There are hypercalls which will return a value that corresponds to a 32-bit
error result - XENMEM_current_reservation, for example - but these are not
likely to be called as part of a multicall. Most of the multicall users
return zero, error, or a count.

-- 
Daniel De Graaf
National Security Agency

On 03/08/2012 19:57, "Daniel De Graaf" <dgdegra@tycho.nsa.gov>
wrote:
> While trying to figure out why a failing component of a multicall did not
> properly return its result, I discovered that multicall results are not
> sign-extended when placed in the unsigned long result field. For hypercalls
> such as do_mmu_update which return a (signed) int, this results in Linux
> incorrectly thinking the hypercall succeeded when it has actually failed
> since arch/x86/xen/multicalls.c uses a signed long for "result"
and checks
> (b->entries[i].result < 0).
> 
> Is this a bug in Xen (using the wrong return type for do_mmu_op and other
> hypercalls) or in Linux (assuming all returns are signed longs)? One or the
> other needs to be changed, because the current setup is silently hiding
> failed memory mapping operations.
I think this is a Xen bug, and we should update all hypercalls to explicitly
return a long. Nice and straightforward.

 -- Keir