From: Sean Christopherson <seanjc@google.com>
To: Gaosheng Cui <cuigaosheng1@huawei.com>
Cc: pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: fix undefined behavior in bit shift for __feature_bit
Date: Mon, 31 Oct 2022 17:42:56 +0000 [thread overview]
Message-ID: <Y2AJIFQlF5C0ozoU@google.com> (raw)
In-Reply-To: <20221031113638.4182263-1-cuigaosheng1@huawei.com>
On Mon, Oct 31, 2022, Gaosheng Cui wrote:
> Shifting signed 32-bit value by 31 bits is undefined, so changing
> significant bit to unsigned. The UBSAN warning calltrace like below:
>
> UBSAN: shift-out-of-bounds in arch/x86/kvm/reverse_cpuid.h:101:11
> left shift of 1 by 31 places cannot be represented in type 'int'
PeterZ is contending that this isn't actually undefined behavior given how the
kernel is compiled[*]. That said, I would be in favor of replacing the open-coded
shift with BIT() to make the code a bit more self-documenting, and that would
naturally fix this maybe-undefined-behavior issue.
[*] https://lore.kernel.org/all/Y1%2FAaJOcgIc%2FINtv@hirez.programming.kicks-ass.net
> ---
> arch/x86/kvm/reverse_cpuid.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/reverse_cpuid.h b/arch/x86/kvm/reverse_cpuid.h
> index a19d473d0184..ebd6b621d3b8 100644
> --- a/arch/x86/kvm/reverse_cpuid.h
> +++ b/arch/x86/kvm/reverse_cpuid.h
> @@ -98,7 +98,7 @@ static __always_inline u32 __feature_bit(int x86_feature)
> x86_feature = __feature_translate(x86_feature);
>
> reverse_cpuid_check(x86_feature / 32);
> - return 1 << (x86_feature & 31);
> + return 1U << (x86_feature & 31);
> }
>
> #define feature_bit(name) __feature_bit(X86_FEATURE_##name)
> --
> 2.25.1
>
next prev parent reply other threads:[~2022-10-31 17:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-31 11:36 [PATCH] KVM: x86: fix undefined behavior in bit shift for __feature_bit Gaosheng Cui
2022-10-31 17:42 ` Sean Christopherson [this message]
2022-10-31 20:27 ` H. Peter Anvin
2022-11-01 2:37 ` cuigaosheng
2022-10-31 20:54 ` Peter Zijlstra
2022-11-02 17:54 ` Paolo Bonzini
2022-11-02 20:56 ` Peter Zijlstra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y2AJIFQlF5C0ozoU@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=cuigaosheng1@huawei.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.