On Fri, 01 Jul 2022 13:22:21 +0100,
Schspa Shi <schspa@xxxxxxxxx> wrote:
>
>
> Marc Zyngier <maz@xxxxxxxxxx> writes:
>
> > On 2022-06-30 17:50, Schspa Shi wrote:
> >> Marc Zyngier <maz@xxxxxxxxxx> writes:
> >>
> >>> On Thu, 30 Jun 2022 17:12:20 +0100,
> >>> Schspa Shi <schspa@xxxxxxxxx> wrote:
> >>>> If the len is 8 bytes, we can't get the correct sign extend for
> >>>> be system.
> >>> I'm afraid you'll have to give me a bit more details.
> >>>
> >>>> Fix the mask type len and the comparison of length.
> >>>> Signed-off-by: Schspa Shi <schspa@xxxxxxxxx>
> >>>> ---
> >>>> arch/arm64/kvm/mmio.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>> diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c
> >>>> index 3dd38a151d2a6..0692f8b18f35c 100644
> >>>> --- a/arch/arm64/kvm/mmio.c
> >>>> +++ b/arch/arm64/kvm/mmio.c
> >>>> @@ -81,8 +81,8 @@ unsigned long kvm_mmio_read_buf(const void
> >>>> *buf, unsigned
> >>>> int len)
> >>>> int kvm_handle_mmio_return(struct kvm_vcpu *vcpu)
> >>>> {
> >>>> unsigned long data;
> >>>> + unsigned long mask;
> >>>> unsigned int len;
> >>>> - int mask;
> >>>> /* Detect an already handled MMIO return */
> >>>> if (unlikely(!vcpu->mmio_needed))
> >>>> @@ -97,7 +97,7 @@ int kvm_handle_mmio_return(struct kvm_vcpu
> >>>> *vcpu)
> >>>> data = kvm_mmio_read_buf(run->mmio.data, len);
> >>>> if (kvm_vcpu_dabt_issext(vcpu) &&
> >>>> - len < sizeof(unsigned long)) {
> >>>> + len <= sizeof(unsigned long)) {
> >>> If you're reading an 8 byte quantity, what is there to
> >>> sign-extend?
> >>> Sign extension only makes sense if what you're reading is
> >>> *smaller*
> >>> than the size of the register you are targeting.
> >>>
> >> Yes, you are correct, sorry for my bad patch.
> >> Please ignore this patch.
> >>
> >>> I must be missing something. And how is that related to running
> >>> BE? BE
> >>> in the host? The guest?
> >> I mean BE is for guest running with BE mode.
> >
> > So what problem did you see? If you have noticed something going
> > wrong, I'd like to get it fixed.
> >
>
> I have running some static code analysis software upon Kernel code.
> Seeing there is possible overflow.
>
> maks << 1U << ((len * 8) -1);
>
> The AI don't know, len is only the value of 1, 2, 4, and make this
> a warnings
>
> I tring to analysis this, but didn't realize the real scenario of
> sign extension, and finally sent this problematic patch.
>
> I do see some uninitialized memory reads (the values are not used
> in the end, just as temporary space for API execution),
> do we need to fix these?
You need to be more descriptive here. What uninitialised reads? In
general, pointing at the code and providing a full description of what
you think is incorrect would really help...
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
