xen/ubsan: Fix UB in type_descriptor declaration

[PATCH] xen/ubsan: Fix UB in type_descriptor declaration

Posted by Andrew Cooper 5 months, 1 week ago

struct type_descriptor is arranged with a NUL terminated string following the
kind/info fields.

The only reason this doesn't trip UBSAN detection itself (on more modern
compilers at least) is because struct type_descriptor is only referenced in
suppressed regions.

Switch the declaration to be a real flexible member.  No functional change.

Fixes: 00fcf4dd8eb4 ("xen/ubsan: Import ubsan implementation from Linux 4.13")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>

For 4.19, and for backport to all reasonable versions.  This bug deserves some
kind of irony award.
---
 xen/common/ubsan/ubsan.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/ubsan/ubsan.h b/xen/common/ubsan/ubsan.h
index a3159040fefb..3db42e75b138 100644
--- a/xen/common/ubsan/ubsan.h
+++ b/xen/common/ubsan/ubsan.h
@@ -10,7 +10,7 @@ enum {
 struct type_descriptor {
 	u16 type_kind;
 	u16 type_info;
-	char type_name[1];
+	char type_name[];
 };
 
 struct source_location {

base-commit: 8b4243a9b560c89bb259db5a27832c253d4bebc7
-- 
2.39.2

Re: [PATCH] xen/ubsan: Fix UB in type_descriptor declaration

Posted by Oleksii K. 5 months ago

On Mon, 2024-06-17 at 18:55 +0100, Andrew Cooper wrote:
> struct type_descriptor is arranged with a NUL terminated string
Should it be NULL instead of NUL?

> following the
> kind/info fields.
> 
> The only reason this doesn't trip UBSAN detection itself (on more
> modern
> compilers at least) is because struct type_descriptor is only
> referenced in
> suppressed regions.
> 
> Switch the declaration to be a real flexible member.  No functional
> change.
> 
> Fixes: 00fcf4dd8eb4 ("xen/ubsan: Import ubsan implementation from
> Linux 4.13")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii
> ---
> CC: George Dunlap <George.Dunlap@citrix.com>
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> 
> For 4.19, and for backport to all reasonable versions.  This bug
> deserves some
> kind of irony award.
> ---
>  xen/common/ubsan/ubsan.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/common/ubsan/ubsan.h b/xen/common/ubsan/ubsan.h
> index a3159040fefb..3db42e75b138 100644
> --- a/xen/common/ubsan/ubsan.h
> +++ b/xen/common/ubsan/ubsan.h
> @@ -10,7 +10,7 @@ enum {
>  struct type_descriptor {
>  	u16 type_kind;
>  	u16 type_info;
> -	char type_name[1];
> +	char type_name[];
>  };
>  
>  struct source_location {
> 
> base-commit: 8b4243a9b560c89bb259db5a27832c253d4bebc7

Re: [PATCH] xen/ubsan: Fix UB in type_descriptor declaration

Posted by Andrew Cooper 5 months ago

On 18/06/2024 9:07 am, Oleksii K. wrote:
> On Mon, 2024-06-17 at 18:55 +0100, Andrew Cooper wrote:
>> struct type_descriptor is arranged with a NUL terminated string
> Should it be NULL instead of NUL?

NULL and NUL can be used interchangeably; they're different spellings
for the same thing.

In the ASCII spec, the character with value 0 is spelt NUL.

>
>> following the
>> kind/info fields.
>>
>> The only reason this doesn't trip UBSAN detection itself (on more
>> modern
>> compilers at least) is because struct type_descriptor is only
>> referenced in
>> suppressed regions.
>>
>> Switch the declaration to be a real flexible member.  No functional
>> change.
>>
>> Fixes: 00fcf4dd8eb4 ("xen/ubsan: Import ubsan implementation from
>> Linux 4.13")
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~Andrew

Re: [PATCH for-4.19] xen/ubsan: Fix UB in type_descriptor declaration

Posted by Jan Beulich 5 months ago

On 17.06.2024 19:55, Andrew Cooper wrote:
> struct type_descriptor is arranged with a NUL terminated string following the
> kind/info fields.
> 
> The only reason this doesn't trip UBSAN detection itself (on more modern
> compilers at least) is because struct type_descriptor is only referenced in
> suppressed regions.
> 
> Switch the declaration to be a real flexible member.  No functional change.
> 
> Fixes: 00fcf4dd8eb4 ("xen/ubsan: Import ubsan implementation from Linux 4.13")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>