[PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Tetsuo Handa]

A kernel built with syzbot's config file reported that

  scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2))

causes uninitialized "save" to be copied.

  ----------
  [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 0
  [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 1
  Console: switching to colour frame buffer device 128x48
  =====================================================
  BUG: KMSAN: uninit-value in do_update_region+0x4b8/0xba0
   do_update_region+0x4b8/0xba0
   update_region+0x40d/0x840
   fbcon_switch+0x3364/0x35e0
   redraw_screen+0xae3/0x18a0
   do_bind_con_driver+0x1cb3/0x1df0
   do_take_over_console+0x11cb/0x13f0
   fbcon_fb_registered+0xacc/0xfd0
   register_framebuffer+0x1179/0x1320
   __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
   drm_fbdev_client_hotplug+0xbea/0xda0
   drm_fbdev_generic_setup+0x65e/0x9d0
   vkms_init+0x9f3/0xc76
   (...snipped...)
  
  Uninit was stored to memory at:
   fbcon_prepare_logo+0x143b/0x1940
   fbcon_init+0x2c1b/0x31c0
   visual_init+0x3e7/0x820
   do_bind_con_driver+0x14a4/0x1df0
   do_take_over_console+0x11cb/0x13f0
   fbcon_fb_registered+0xacc/0xfd0
   register_framebuffer+0x1179/0x1320
   __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
   drm_fbdev_client_hotplug+0xbea/0xda0
   drm_fbdev_generic_setup+0x65e/0x9d0
   vkms_init+0x9f3/0xc76
   (...snipped...)
  
  Uninit was created at:
   __kmem_cache_alloc_node+0xb69/0x1020
   __kmalloc+0x379/0x680
   fbcon_prepare_logo+0x704/0x1940
   fbcon_init+0x2c1b/0x31c0
   visual_init+0x3e7/0x820
   do_bind_con_driver+0x14a4/0x1df0
   do_take_over_console+0x11cb/0x13f0
   fbcon_fb_registered+0xacc/0xfd0
   register_framebuffer+0x1179/0x1320
   __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
   drm_fbdev_client_hotplug+0xbea/0xda0
   drm_fbdev_generic_setup+0x65e/0x9d0
   vkms_init+0x9f3/0xc76
   (...snipped...)
  
  CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc4-00356-g8f2975c2bb4c #924
  Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
  ----------

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/video/fbdev/core/fbcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
index 098b62f7b701..c0143d38df83 100644
--- a/drivers/video/fbdev/core/fbcon.c
+++ b/drivers/video/fbdev/core/fbcon.c
@@ -577,7 +577,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
 		if (scr_readw(r) != vc->vc_video_erase_char)
 			break;
 	if (r != q && new_rows >= rows + logo_lines) {
-		save = kmalloc(array3_size(logo_lines, new_cols, 2),
+		save = kzalloc(array3_size(logo_lines, new_cols, 2),
 			       GFP_KERNEL);
 		if (save) {
 			int i = min(cols, new_cols);
-- 
2.34.1

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Daniel Vetter]

On Fri, Nov 18, 2022 at 12:27:58AM +0900, Tetsuo Handa wrote:
> A kernel built with syzbot's config file reported that
> 
>   scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2))
> 
> causes uninitialized "save" to be copied.
> 
>   ----------
>   [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 0
>   [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 1
>   Console: switching to colour frame buffer device 128x48
>   =====================================================
>   BUG: KMSAN: uninit-value in do_update_region+0x4b8/0xba0
>    do_update_region+0x4b8/0xba0
>    update_region+0x40d/0x840
>    fbcon_switch+0x3364/0x35e0
>    redraw_screen+0xae3/0x18a0
>    do_bind_con_driver+0x1cb3/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   Uninit was stored to memory at:
>    fbcon_prepare_logo+0x143b/0x1940
>    fbcon_init+0x2c1b/0x31c0
>    visual_init+0x3e7/0x820
>    do_bind_con_driver+0x14a4/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   Uninit was created at:
>    __kmem_cache_alloc_node+0xb69/0x1020
>    __kmalloc+0x379/0x680
>    fbcon_prepare_logo+0x704/0x1940
>    fbcon_init+0x2c1b/0x31c0
>    visual_init+0x3e7/0x820
>    do_bind_con_driver+0x14a4/0x1df0
>    do_take_over_console+0x11cb/0x13f0
>    fbcon_fb_registered+0xacc/0xfd0
>    register_framebuffer+0x1179/0x1320
>    __drm_fb_helper_initial_config_and_unlock+0x23ad/0x2b40
>    drm_fbdev_client_hotplug+0xbea/0xda0
>    drm_fbdev_generic_setup+0x65e/0x9d0
>    vkms_init+0x9f3/0xc76
>    (...snipped...)
>   
>   CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc4-00356-g8f2975c2bb4c #924
>   Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
>   ----------
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks for your patch, pushed to drm-misc-fixes.
-Daniel

> ---
>  drivers/video/fbdev/core/fbcon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c
> index 098b62f7b701..c0143d38df83 100644
> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -577,7 +577,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
>  		if (scr_readw(r) != vc->vc_video_erase_char)
>  			break;
>  	if (r != q && new_rows >= rows + logo_lines) {
> -		save = kmalloc(array3_size(logo_lines, new_cols, 2),
> +		save = kzalloc(array3_size(logo_lines, new_cols, 2),
>  			       GFP_KERNEL);
>  		if (save) {
>  			int i = min(cols, new_cols);
> -- 
> 2.34.1

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Geert Uytterhoeven]

Hi Handa-san,

On Thu, Nov 17, 2022 at 4:32 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> A kernel built with syzbot's config file reported that
>
>   scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2))
>
> causes uninitialized "save" to be copied.

> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Thanks for your patch, which is now commit a6a00d7e8ffd78d1
("fbcon: Use kzalloc() in fbcon_prepare_logo()") in v6.1-rc7,
and which is being backported to stable.

> --- a/drivers/video/fbdev/core/fbcon.c
> +++ b/drivers/video/fbdev/core/fbcon.c
> @@ -577,7 +577,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct fb_info *info,
>                 if (scr_readw(r) != vc->vc_video_erase_char)
>                         break;
>         if (r != q && new_rows >= rows + logo_lines) {
> -               save = kmalloc(array3_size(logo_lines, new_cols, 2),
> +               save = kzalloc(array3_size(logo_lines, new_cols, 2),
>                                GFP_KERNEL);
>                 if (save) {
>                         int i = min(cols, new_cols);

The next line is:

                        scr_memsetw(save, erase,
array3_size(logo_lines, new_cols, 2));

So how can this turn out to be uninitialized later below?

                scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2));

What am I missing?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Tetsuo Handa]

On 2022/12/15 18:36, Geert Uytterhoeven wrote:
> The next line is:
> 
>         scr_memsetw(save, erase, array3_size(logo_lines, new_cols, 2));
> 
> So how can this turn out to be uninitialized later below?
> 
>         scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2));
> 
> What am I missing?

Good catch. It turned out that this was a KMSAN problem (i.e. a false positive report).

On x86_64, scr_memsetw() is implemented as

        static inline void scr_memsetw(u16 *s, u16 c, unsigned int count)
        {
                memset16(s, c, count / 2);
        }

and memset16() is implemented as

        static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
        {
        	long d0, d1;
        	asm volatile("rep\n\t"
        		     "stosw"
        		     : "=&c" (d0), "=&D" (d1)
        		     : "a" (v), "1" (s), "0" (n)
        		     : "memory");
        	return s;
        }

. Plain memset() in arch/x86/include/asm/string_64.h is redirected to __msan_memset()
but memsetXX() are not redirected to __msan_memsetXX(). That is, memory initialization
via memsetXX() results in KMSAN's shadow memory being not updated.

KMSAN folks, how should we fix this problem?
Redirect assembly-implemented memset16(size) to memset(size*2) if KMSAN is enabled?

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Alexander Potapenko]

On Fri, Dec 16, 2022 at 3:03 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> On 2022/12/15 18:36, Geert Uytterhoeven wrote:
> > The next line is:
> >
> >         scr_memsetw(save, erase, array3_size(logo_lines, new_cols, 2));
> >
> > So how can this turn out to be uninitialized later below?
> >
> >         scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2));
> >
> > What am I missing?
>
> Good catch. It turned out that this was a KMSAN problem (i.e. a false positive report).
>
> On x86_64, scr_memsetw() is implemented as
>
>         static inline void scr_memsetw(u16 *s, u16 c, unsigned int count)
>         {
>                 memset16(s, c, count / 2);
>         }
>
> and memset16() is implemented as
>
>         static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
>         {
>                 long d0, d1;
>                 asm volatile("rep\n\t"
>                              "stosw"
>                              : "=&c" (d0), "=&D" (d1)
>                              : "a" (v), "1" (s), "0" (n)
>                              : "memory");
>                 return s;
>         }
>
> . Plain memset() in arch/x86/include/asm/string_64.h is redirected to __msan_memset()
> but memsetXX() are not redirected to __msan_memsetXX(). That is, memory initialization
> via memsetXX() results in KMSAN's shadow memory being not updated.
>
> KMSAN folks, how should we fix this problem?
> Redirect assembly-implemented memset16(size) to memset(size*2) if KMSAN is enabled?
>

I think the easiest way to fix it would be disable memsetXX asm
implementations by something like:

-------------------------------------------------------------------------------------------------
diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
index 888731ccf1f67..5fb330150a7d1 100644
--- a/arch/x86/include/asm/string_64.h
+++ b/arch/x86/include/asm/string_64.h
@@ -33,6 +33,7 @@ void *memset(void *s, int c, size_t n);
 #endif
 void *__memset(void *s, int c, size_t n);

+#if !defined(__SANITIZE_MEMORY__)
 #define __HAVE_ARCH_MEMSET16
 static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
 {
@@ -68,6 +69,7 @@ static inline void *memset64(uint64_t *s, uint64_t
v, size_t n)
                     : "memory");
        return s;
 }
+#endif

 #define __HAVE_ARCH_MEMMOVE
 #if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
-------------------------------------------------------------------------------------------------

This way we'll just pick the existing C implementations instead of
reinventing them.


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Daniel Vetter]

On Fri, Dec 16, 2022 at 04:52:14PM +0100, Alexander Potapenko wrote:
> On Fri, Dec 16, 2022 at 3:03 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >
> > On 2022/12/15 18:36, Geert Uytterhoeven wrote:
> > > The next line is:
> > >
> > >         scr_memsetw(save, erase, array3_size(logo_lines, new_cols, 2));
> > >
> > > So how can this turn out to be uninitialized later below?
> > >
> > >         scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2));
> > >
> > > What am I missing?
> >
> > Good catch. It turned out that this was a KMSAN problem (i.e. a false positive report).
> >
> > On x86_64, scr_memsetw() is implemented as
> >
> >         static inline void scr_memsetw(u16 *s, u16 c, unsigned int count)
> >         {
> >                 memset16(s, c, count / 2);
> >         }
> >
> > and memset16() is implemented as
> >
> >         static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
> >         {
> >                 long d0, d1;
> >                 asm volatile("rep\n\t"
> >                              "stosw"
> >                              : "=&c" (d0), "=&D" (d1)
> >                              : "a" (v), "1" (s), "0" (n)
> >                              : "memory");
> >                 return s;
> >         }
> >
> > . Plain memset() in arch/x86/include/asm/string_64.h is redirected to __msan_memset()
> > but memsetXX() are not redirected to __msan_memsetXX(). That is, memory initialization
> > via memsetXX() results in KMSAN's shadow memory being not updated.
> >
> > KMSAN folks, how should we fix this problem?
> > Redirect assembly-implemented memset16(size) to memset(size*2) if KMSAN is enabled?
> >
> 
> I think the easiest way to fix it would be disable memsetXX asm
> implementations by something like:
> 
> -------------------------------------------------------------------------------------------------
> diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
> index 888731ccf1f67..5fb330150a7d1 100644
> --- a/arch/x86/include/asm/string_64.h
> +++ b/arch/x86/include/asm/string_64.h
> @@ -33,6 +33,7 @@ void *memset(void *s, int c, size_t n);
>  #endif
>  void *__memset(void *s, int c, size_t n);
> 
> +#if !defined(__SANITIZE_MEMORY__)
>  #define __HAVE_ARCH_MEMSET16
>  static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
>  {
> @@ -68,6 +69,7 @@ static inline void *memset64(uint64_t *s, uint64_t
> v, size_t n)
>                      : "memory");
>         return s;
>  }
> +#endif

So ... what should I do here? Can someone please send me a revert or patch
to apply. I don't think I should do this, since I already tossed my credit
for not looking at stuff carefully enough into the wind :-)
-Daniel

> 
>  #define __HAVE_ARCH_MEMMOVE
>  #if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
> -------------------------------------------------------------------------------------------------
> 
> This way we'll just pick the existing C implementations instead of
> reinventing them.
> 
> 
> -- 
> Alexander Potapenko
> Software Engineer
> 
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
> 
> Geschäftsführer: Paul Manicle, Liana Sebastian
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Tetsuo Handa]

On 2023/01/05 20:54, Daniel Vetter wrote:
>>> . Plain memset() in arch/x86/include/asm/string_64.h is redirected to __msan_memset()
>>> but memsetXX() are not redirected to __msan_memsetXX(). That is, memory initialization
>>> via memsetXX() results in KMSAN's shadow memory being not updated.
>>>
>>> KMSAN folks, how should we fix this problem?
>>> Redirect assembly-implemented memset16(size) to memset(size*2) if KMSAN is enabled?
>>>
>>
>> I think the easiest way to fix it would be disable memsetXX asm
>> implementations by something like:
>>
>> -------------------------------------------------------------------------------------------------
>> diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
>> index 888731ccf1f67..5fb330150a7d1 100644
>> --- a/arch/x86/include/asm/string_64.h
>> +++ b/arch/x86/include/asm/string_64.h
>> @@ -33,6 +33,7 @@ void *memset(void *s, int c, size_t n);
>>  #endif
>>  void *__memset(void *s, int c, size_t n);
>>
>> +#if !defined(__SANITIZE_MEMORY__)
>>  #define __HAVE_ARCH_MEMSET16
>>  static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
>>  {
>> @@ -68,6 +69,7 @@ static inline void *memset64(uint64_t *s, uint64_t
>> v, size_t n)
>>                      : "memory");
>>         return s;
>>  }
>> +#endif
> 
> So ... what should I do here? Can someone please send me a revert or patch
> to apply. I don't think I should do this, since I already tossed my credit
> for not looking at stuff carefully enough into the wind :-)
> -Daniel
> 
>>
>>  #define __HAVE_ARCH_MEMMOVE
>>  #if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
>> -------------------------------------------------------------------------------------------------
>>
>> This way we'll just pick the existing C implementations instead of
>> reinventing them.
>>

I'd like to avoid touching per-arch asm/string.h files if possible.

Can't we do like below (i.e. keep asm implementations as-is, but
automatically redirect to __msan_memset()) ? If yes, we could move all
__msan_*() redirection from per-arch asm/string.h files to the common
linux/string.h file?

diff --git a/include/linux/string.h b/include/linux/string.h
index c062c581a98b..403813b04e00 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -360,4 +360,15 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
 	return strncmp(str, prefix, len) == 0 ? len : 0;
 }
 
+#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
+#undef memset
+#define memset(dest, src, count) __msan_memset((dest), (src), (count))
+#undef memset16
+#define memset16(dest, src, count) __msan_memset((dest), (src), (count) << 1)
+#undef memset32
+#define memset32(dest, src, count) __msan_memset((dest), (src), (count) << 2)
+#undef memset64
+#define memset64(dest, src, count) __msan_memset((dest), (src), (count) << 3)
+#endif
+
 #endif /* _LINUX_STRING_H_ */

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Daniel Vetter]

On Thu, Jan 05, 2023 at 10:17:24PM +0900, Tetsuo Handa wrote:
> On 2023/01/05 20:54, Daniel Vetter wrote:
> >>> . Plain memset() in arch/x86/include/asm/string_64.h is redirected to __msan_memset()
> >>> but memsetXX() are not redirected to __msan_memsetXX(). That is, memory initialization
> >>> via memsetXX() results in KMSAN's shadow memory being not updated.
> >>>
> >>> KMSAN folks, how should we fix this problem?
> >>> Redirect assembly-implemented memset16(size) to memset(size*2) if KMSAN is enabled?
> >>>
> >>
> >> I think the easiest way to fix it would be disable memsetXX asm
> >> implementations by something like:
> >>
> >> -------------------------------------------------------------------------------------------------
> >> diff --git a/arch/x86/include/asm/string_64.h b/arch/x86/include/asm/string_64.h
> >> index 888731ccf1f67..5fb330150a7d1 100644
> >> --- a/arch/x86/include/asm/string_64.h
> >> +++ b/arch/x86/include/asm/string_64.h
> >> @@ -33,6 +33,7 @@ void *memset(void *s, int c, size_t n);
> >>  #endif
> >>  void *__memset(void *s, int c, size_t n);
> >>
> >> +#if !defined(__SANITIZE_MEMORY__)
> >>  #define __HAVE_ARCH_MEMSET16
> >>  static inline void *memset16(uint16_t *s, uint16_t v, size_t n)
> >>  {
> >> @@ -68,6 +69,7 @@ static inline void *memset64(uint64_t *s, uint64_t
> >> v, size_t n)
> >>                      : "memory");
> >>         return s;
> >>  }
> >> +#endif
> > 
> > So ... what should I do here? Can someone please send me a revert or patch
> > to apply. I don't think I should do this, since I already tossed my credit
> > for not looking at stuff carefully enough into the wind :-)
> > -Daniel
> > 
> >>
> >>  #define __HAVE_ARCH_MEMMOVE
> >>  #if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
> >> -------------------------------------------------------------------------------------------------
> >>
> >> This way we'll just pick the existing C implementations instead of
> >> reinventing them.
> >>
> 
> I'd like to avoid touching per-arch asm/string.h files if possible.
> 
> Can't we do like below (i.e. keep asm implementations as-is, but
> automatically redirect to __msan_memset()) ? If yes, we could move all
> __msan_*() redirection from per-arch asm/string.h files to the common
> linux/string.h file?

Oh I was more asking about the fbdev patch. This here sounds a lot more
something that needs to be discussed with kmsan people, that's definitely
not my area.
-Daniel

> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index c062c581a98b..403813b04e00 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -360,4 +360,15 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
>  	return strncmp(str, prefix, len) == 0 ? len : 0;
>  }
>  
> +#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
> +#undef memset
> +#define memset(dest, src, count) __msan_memset((dest), (src), (count))
> +#undef memset16
> +#define memset16(dest, src, count) __msan_memset((dest), (src), (count) << 1)
> +#undef memset32
> +#define memset32(dest, src, count) __msan_memset((dest), (src), (count) << 2)
> +#undef memset64
> +#define memset64(dest, src, count) __msan_memset((dest), (src), (count) << 3)
> +#endif
> +
>  #endif /* _LINUX_STRING_H_ */
> 
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Tetsuo Handa]

On 2023/01/05 22:22, Daniel Vetter wrote:
> Oh I was more asking about the fbdev patch. This here sounds a lot more
> something that needs to be discussed with kmsan people, that's definitely
> not my area.
> -Daniel

Commit a6a00d7e8ffd ("fbcon: Use kzalloc() in fbcon_prepare_logo()") was
redundant but not reverting that commit is harmless. You don't need to
worry about this problem. This is a problem for KMSAN people.

Re: [PATCH] fbcon: Use kzalloc() in fbcon_prepare_logo()

[Alexander Potapenko]

>
> I'd like to avoid touching per-arch asm/string.h files if possible.
>
> Can't we do like below (i.e. keep asm implementations as-is, but
> automatically redirect to __msan_memset()) ? If yes, we could move all
> __msan_*() redirection from per-arch asm/string.h files to the common
> linux/string.h file?
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index c062c581a98b..403813b04e00 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -360,4 +360,15 @@ static __always_inline size_t str_has_prefix(const char *str, const char *prefix
>         return strncmp(str, prefix, len) == 0 ? len : 0;
>  }
>
> +#if defined(__SANITIZE_MEMORY__) && defined(__NO_FORTIFY)
> +#undef memset
> +#define memset(dest, src, count) __msan_memset((dest), (src), (count))
> +#undef memset16
> +#define memset16(dest, src, count) __msan_memset((dest), (src), (count) << 1)
> +#undef memset32
> +#define memset32(dest, src, count) __msan_memset((dest), (src), (count) << 2)
> +#undef memset64
> +#define memset64(dest, src, count) __msan_memset((dest), (src), (count) << 3)
> +#endif

The problem with this approach is that it can only work for
memset/memcpy/memmove, whereas any function that is implemented in
lib/string.c may require undefining the respective __HAVE_ARCH_FNAME
so that KMSAN can instrument it.