Hi

Am 30.07.25 um 14:51 schrieb Gote, Nitin R:
Hi,

The patch has been reviewed and approved.
Could someone please help to merge it?

Merged into drm-misc-fixes now, sorry for the delay.

Best regards
Thomas


Thank you,
Nitin

-----Original Message-----
From: Gote, Nitin R
Sent: Thursday, July 24, 2025 10:36 AM
To: Thomas Zimmermann <tzimmerm...@suse.de>
Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Shyti, 
Andi
<andi.sh...@intel.com>
Subject: RE: [PATCH] iosys-map: Fix undefined behavior in iosys_map_clear()

Hi Thomas,

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.g...@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 <tzimmerm...@suse.de>

Best regards
Thomas

Thank you for the review.
Could you please help to merge this patch?

- Nitin

Reviewed-by: Andi Shyti <andi.sh...@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)

--
--
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)


Reply via email to