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

[Gote, Nitin R]

Hi, 

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

Thank you,
Nitin

> -----Original Message-----
> From: Gote, Nitin R
> Sent: Thursday, July 24, 2025 10:36 AM
> To: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org; Shyti, Andi
> <Andi.Shyti at 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.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
> >
> 
> Thank you for the review.
> Could you please help to merge this patch?
> 
> - Nitin
> 
> > >
> > > 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)