diff mbox series

[4.19,174/229] x86/entry: Work around Clang __bdos() bug

Message ID 20221024113004.718917343@linuxfoundation.org
State New
Headers
Series None |

Commit Message

Greg KH Oct. 24, 2022, 11:31 a.m. UTC 
  From: Kees Cook <keescook@chromium.org>

[ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]

Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
offset. Work around this by using a direct assignment of an empty
instance. Avoids this warning:

../include/linux/fortify-string.h:309:4: warning: call to __write_overflow_field declared with 'warn
ing' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wat
tribute-warning]
                        __write_overflow_field(p_size_field, size);
                        ^

which was isolated to the memset() call in xen_load_idt().

Note that this looks very much like another bug that was worked around:
https://github.com/ClangBuiltLinux/linux/issues/1592

Cc: Juergen Gross <jgross@suse.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: xen-devel@lists.xenproject.org
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Link: https://lore.kernel.org/lkml/41527d69-e8ab-3f86-ff37-6b298c01d5bc@oracle.com
Signed-off-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 arch/x86/xen/enlighten_pv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Pavel Machek Oct. 24, 2022, 5:41 p.m. UTC | #1 
Hi!

> From: Kees Cook <keescook@chromium.org>
> 
> [ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]
> 
> Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
> and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
> offset. Work around this by using a direct assignment of an empty
> instance. Avoids this warning:
> 
> ../include/linux/fortify-string.h:309:4: warning: call to __write_overflow_field declared with 'warn
> ing' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wat
> tribute-warning]
>                         __write_overflow_field(p_size_field, size);
>                         ^
> 
> which was isolated to the memset() call in xen_load_idt().
> 
> Note that this looks very much like another bug that was worked around:
> https://github.com/ClangBuiltLinux/linux/issues/1592

We don't have CONFIG_UBSAN_BOUNDS in 4.19, so maybe we don't need this
one?

Best regards,
								Pavel
								
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -752,6 +752,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
>  {
>  	static DEFINE_SPINLOCK(lock);
>  	static struct trap_info traps[257];
> +	static const struct trap_info zero = { };
>  	unsigned out;
>  
>  	trace_xen_cpu_load_idt(desc);
> @@ -761,7 +762,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
>  	memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
>  
>  	out = xen_convert_trap_info(desc, traps, false);
> -	memset(&traps[out], 0, sizeof(traps[0]));
> +	traps[out] = zero;
>  
>  	xen_mc_flush();
>  	if (HYPERVISOR_set_trap_table(traps))
> -- 
> 2.35.1
> 
>
Greg KH Oct. 25, 2022, 12:47 p.m. UTC | #2 
On Mon, Oct 24, 2022 at 07:41:27PM +0200, Pavel Machek wrote:
> Hi!
> 
> > From: Kees Cook <keescook@chromium.org>
> > 
> > [ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]
> > 
> > Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
> > and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
> > offset. Work around this by using a direct assignment of an empty
> > instance. Avoids this warning:
> > 
> > ../include/linux/fortify-string.h:309:4: warning: call to __write_overflow_field declared with 'warn
> > ing' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wat
> > tribute-warning]
> >                         __write_overflow_field(p_size_field, size);
> >                         ^
> > 
> > which was isolated to the memset() call in xen_load_idt().
> > 
> > Note that this looks very much like another bug that was worked around:
> > https://github.com/ClangBuiltLinux/linux/issues/1592
> 
> We don't have CONFIG_UBSAN_BOUNDS in 4.19, so maybe we don't need this
> one?

Good point, I'll drop this from 5.4.y and older now, thanks.

greg k-h
diff mbox series

Patch

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 8f1ff8dad2ce..04bfd9c3987b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -752,6 +752,7 @@  static void xen_load_idt(const struct desc_ptr *desc)
 {
 	static DEFINE_SPINLOCK(lock);
 	static struct trap_info traps[257];
+	static const struct trap_info zero = { };
 	unsigned out;
 
 	trace_xen_cpu_load_idt(desc);
@@ -761,7 +762,7 @@  static void xen_load_idt(const struct desc_ptr *desc)
 	memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc));
 
 	out = xen_convert_trap_info(desc, traps, false);
-	memset(&traps[out], 0, sizeof(traps[0]));
+	traps[out] = zero;
 
 	xen_mc_flush();
 	if (HYPERVISOR_set_trap_table(traps))