All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Sean Christopherson <seanjc@google.com>,
	Yang Li <yang.lee@linux.alibaba.com>
Cc: pbonzini@redhat.com, wanpengli@tencent.com, jmattson@google.com,
	joro@8bytes.org, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, x86@kernel.org, hpa@zytor.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] KVM: x86: Fix potential memory access error
Date: Thu, 01 Apr 2021 11:08:13 +0200	[thread overview]
Message-ID: <87mtuis77m.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <YGS6XS87HYJdVPFQ@google.com>

Sean Christopherson <seanjc@google.com> writes:

> On Wed, Mar 31, 2021, Yang Li wrote:
>> Using __set_bit() to set a bit in an integer is not a good idea, since
>> the function expects an unsigned long as argument, which can be 64bit wide.
>> Coverity reports this problem as
>> 
>> High:Out-of-bounds access(INCOMPATIBLE_CAST)
>> CWE119: Out-of-bounds access to a scalar
>> Pointer "&vcpu->arch.regs_avail" points to an object whose effective
>> type is "unsigned int" (32 bits, unsigned) but is dereferenced as a
>> wider "unsigned long" (64 bits, unsigned). This may lead to memory
>> corruption.
>> 
>> /home/heyuan.shy/git-repo/linux/arch/x86/kvm/kvm_cache_regs.h:
>> kvm_register_is_available
>> 
>> Just use BIT instead.
>
> Meh, we're hosed either way.  Using BIT() will either result in undefined
> behavior due to SHL shifting beyond the size of a u64, or setting random bits
> if the truncated shift ends up being less than 63.
>

A stupid question: why can't we just make 'regs_avail'/'regs_dirty'
'unsigned long' and drop a bunch of '(unsigned long *)' casts? 

-- 
Vitaly


  reply	other threads:[~2021-04-01  9:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-31  9:15 [PATCH] KVM: x86: Fix potential memory access error Yang Li
2021-03-31 18:07 ` Sean Christopherson
2021-04-01  9:08   ` Vitaly Kuznetsov [this message]
2021-04-01 16:22     ` Sean Christopherson

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=87mtuis77m.fsf@vitty.brq.redhat.com \
    --to=vkuznets@redhat.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    --cc=yang.lee@linux.alibaba.com \
    /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.