[PATCH] x86/mwait-idle: fix ubsan warning

Tamas K Lengyel posted 1 patch 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/f0ac3890b5e2e1e98bfd3fe5fffcf3c3c031e12c.1704388276.git.tamas.lengyel@intel.com
xen/arch/x86/include/asm/mwait.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] x86/mwait-idle: fix ubsan warning
Posted by Tamas K Lengyel 4 months ago
Fix warning:
(XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
(XEN) left shift of 15 by 28 places cannot be represented in type 'int'

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
---
 xen/arch/x86/include/asm/mwait.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/mwait.h b/xen/arch/x86/include/asm/mwait.h
index f377d9f..9298f98 100644
--- a/xen/arch/x86/include/asm/mwait.h
+++ b/xen/arch/x86/include/asm/mwait.h
@@ -4,7 +4,7 @@
 #include <xen/types.h>
 
 #define MWAIT_SUBSTATE_MASK		0xf
-#define MWAIT_CSTATE_MASK		0xf
+#define MWAIT_CSTATE_MASK		0xfU
 #define MWAIT_SUBSTATE_SIZE		4
 
 #define CPUID_MWAIT_LEAF		5
-- 
2.34.1
Re: [PATCH] x86/mwait-idle: fix ubsan warning
Posted by Jan Beulich 4 months ago
On 04.01.2024 18:13, Tamas K Lengyel wrote:
> Fix warning:
> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")

No matter that I appreciate the change, I think this wants fixing by a
patch to the (Linux) original, which we'd then import (like we do for
other changes, including the one referenced by the Fixes: tag).

Jan
Re: [PATCH] x86/mwait-idle: fix ubsan warning
Posted by Tamas K Lengyel 4 months ago
On Fri, Jan 5, 2024 at 2:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.01.2024 18:13, Tamas K Lengyel wrote:
> > Fix warning:
> > (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> > (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> > Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
>
> No matter that I appreciate the change, I think this wants fixing by a
> patch to the (Linux) original, which we'd then import (like we do for
> other changes, including the one referenced by the Fixes: tag).

Feel free to submit it to other projects if the same issue applies to
them. I only ran into this with Xen and can only test it with Xen.

Tamas
Re: [PATCH] x86/mwait-idle: fix ubsan warning
Posted by Andrew Cooper 4 months ago
On 05/01/2024 4:09 pm, Tamas K Lengyel wrote:
> On Fri, Jan 5, 2024 at 2:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>> On 04.01.2024 18:13, Tamas K Lengyel wrote:
>>> Fix warning:
>>> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
>>> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
>>> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")
>> No matter that I appreciate the change, I think this wants fixing by a
>> patch to the (Linux) original, which we'd then import (like we do for
>> other changes, including the one referenced by the Fixes: tag).
> Feel free to submit it to other projects if the same issue applies to
> them. I only ran into this with Xen and can only test it with Xen.

Linux is affected by this, but a fix to Linux won't apply to Xen because
Xen already diverged from Linux in this function.

~Andrew

Re: [PATCH] x86/mwait-idle: fix ubsan warning
Posted by Andrew Cooper 4 months ago
On 04/01/2024 5:13 pm, Tamas K Lengyel wrote:
> Fix warning:
> (XEN) UBSAN: Undefined behaviour in arch/x86/cpu/mwait-idle.c:1300:44
> (XEN) left shift of 15 by 28 places cannot be represented in type 'int'
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
> Fixes: 5a211704e88 ("mwait-idle: prevent SKL-H boot failure when C8+C9+C10 enabled")

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

It's perhaps worth saying this is in sklh_idle_state_table_update()
which is why it only manifests on a single CPU.  Happy to adjust on commit.

All other uses of this constant shift right first, and then mask, hence
why they don't trip UBSAN.

~Andrew