[PATCH] iosys-map: Fix undefined behavior in iosys_map_clear()

[Thomas Zimmermann]

Hi

Am 18.07.25 um 16:47 schrieb Andi Shyti:
> Hi Nitin,
>
> On Fri, Jul 18, 2025 at 04:20:51PM +0530, Nitin Gote wrote:
>> The current iosys_map_clear() implementation reads the potentially
>> uninitialized 'is_iomem' boolean field to decide which union member
>> to clear. This causes undefined behavior when called on uninitialized
>> structures, as 'is_iomem' may contain garbage values like 0xFF.
>>
>> UBSAN detects this as:
>>      UBSAN: invalid-load in include/linux/iosys-map.h:267
>>      load of value 255 is not a valid value for type '_Bool'
>>
>> Fix by unconditionally clearing the entire structure with memset(),
>> eliminating the need to read uninitialized data and ensuring all
>> fields are set to known good values.
>>
>> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14639
>> Fixes: 01fd30da0474 ("dma-buf: Add struct dma-buf-map for storing struct dma_buf.vaddr_ptr")
>> Signed-off-by: Nitin Gote <nitin.r.gote at intel.com>
> +Thomas and the dri-devel mailing list.
>
> In any case, your patch makes sense to me:

The call to iosys_map_clear() is at

https://elixir.bootlin.com/linux/v6.15.6/source/drivers/dma-buf/dma-buf.c#L1571

It's a defensive measure for cases where the caller reads the returned 
map address when it was never initialized by the vmap implementation. 
I'm not a big fan of memset(), but OK. Preferably, iosys_map_clear() 
would simply set vaddr = NULL and is_iomem = false. Anyway,

Reviewed-by: Thomas Zimmermann <tzimmermann at suse.de>

Best regards
Thomas

>
> Reviewed-by: Andi Shyti <andi.shyti at linux.intel.com>
>
> Andi
>
>> ---
>>   include/linux/iosys-map.h | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>> index 4696abfd311c..3e85afe794c0 100644
>> --- a/include/linux/iosys-map.h
>> +++ b/include/linux/iosys-map.h
>> @@ -264,12 +264,7 @@ static inline bool iosys_map_is_set(const struct iosys_map *map)
>>    */
>>   static inline void iosys_map_clear(struct iosys_map *map)
>>   {
>> -	if (map->is_iomem) {
>> -		map->vaddr_iomem = NULL;
>> -		map->is_iomem = false;
>> -	} else {
>> -		map->vaddr = NULL;
>> -	}
>> +	memset(map, 0, sizeof(*map));
>>   }
>>   
>>   /**
>> -- 
>> 2.25.1

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)