Re: [PATCH] xsm/flask: Fix undefined behaviour in avc_dump_av()

[Dmytro Prokopchuk1 <dmytro_prokopchuk1@xxxxxxxx>]

On 5/1/26 17:27, Andrew Cooper wrote:
> On 01/05/2026 3:17 pm, Dmytro Prokopchuk1 wrote:
>> When booting Xen with CONFIG_USBAN=y and CONFIG_XSM_FLASK=y,
>> UBSAN reports undefined behaviour in avc_dump_av() due to a left
>> shift on a signed int:
>>
>> (XEN) [    1.104348] 
>> ================================================================================
>> (XEN) [    1.105096] UBSAN: Undefined behaviour in xsm/flask/avc.c:184:14
>> (XEN) [    1.106052] left shift of 1073741824 by 1 places cannot be 
>> represented in type 'int'
>> (XEN) [    1.107546] Xen WARN at common/ubsan/ubsan.c:176
>> (XEN) [    1.108295] ----[ Xen-4.21.1  arm64  debug=y ubsan=y  Not tainted 
>> ]----
>> (XEN) [    1.108848] CPU:    0
>> (XEN) [    1.109147] PC:     00000a00002f64fc 
>> ubsan.c#ubsan_epilogue+0x10/0xd4
>> [...]
>> (XEN) [    1.146320] Xen call trace:
>> (XEN) [    1.146663]    [<00000a00002f64fc>] 
>> ubsan.c#ubsan_epilogue+0x10/0xd4 (PC)
>> (XEN) [    1.147227]    [<00000a00002f7bc4>] 
>> __ubsan_handle_shift_out_of_bounds+0x1a0/0x290 (LR)
>> (XEN) [    1.147868]
>> (XEN) [    1.148177] 
>> ================================================================================
>>
>> This can be solved by making 'perm' an unsigned 32-bit type (u32).
>>
>> Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
>> ---
>> Test CI pipeline:
>> https://gitlab.com/xen-project/people/dimaprkp4k/xen/-/pipelines/2493649109
>> ---
>>   xen/xsm/flask/avc.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
>> index 3d39e55cae..9c3ffdc070 100644
>> --- a/xen/xsm/flask/avc.c
>> +++ b/xen/xsm/flask/avc.c
>> @@ -152,7 +152,8 @@ static void __attribute__ ((format (printf, 2, 3)))
>>    */
>>   static void avc_dump_av(struct avc_dump_buf *buf, u16 tclass, u32 av)
>>   {
>> -    int i, i2, perm;
>> +    int i, i2;
>> +    u32 perm;
>>
>>       if ( av == 0 )
>>       {
>
> The fix is fine, but wants to be uint32_t.  (The existing code is
> already inconsistent, and wants fixing up towards Xen's preferred style.)
>
> Can be fixed on commit.
>
> ~Andrew

Hello,

I'm fine with that.

Thanks, Dmytro.