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

Dmytro Prokopchuk1 posted 1 patch 3 days, 18 hours ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/43fc4b4ed45858b2cebbc37bbbf3b70e664a0661.1777642449.git.dmytro._5Fprokopchuk1@epam.com
xen/xsm/flask/avc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
[PATCH] xsm/flask: Fix undefined behaviour in avc_dump_av()
Posted by Dmytro Prokopchuk1 3 days, 18 hours ago
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@epam.com>
---
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 )
     {
-- 
2.43.0
Re: [PATCH] xsm/flask: Fix undefined behaviour in avc_dump_av()
Posted by Andrew Cooper 3 days, 18 hours ago
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@epam.com>
> ---
> 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

Re: [PATCH] xsm/flask: Fix undefined behaviour in avc_dump_av()
Posted by Dmytro Prokopchuk1 3 days, 1 hour ago

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@epam.com>
>> ---
>> 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.