| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 27 Oct 2017 15:43:26 +0000
From: gengdongjiu <gengdongjiu@...wei.com>
To: Marc Zyngier <marc.zyngier@....com>
CC: "christoffer.dall@...aro.org" <christoffer.dall@...aro.org>,
"linux@...linux.org.uk" <linux@...linux.org.uk>,
"james.morse@....com" <james.morse@....com>,
"tbaicar@...eaurora.org" <tbaicar@...eaurora.org>,
"catalin.marinas@....com" <catalin.marinas@....com>,
"will.deacon@....com" <will.deacon@....com>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: [PATCH v2] KVM: arm/arm64: Fix external abort type matching
>
> On Thu, Oct 26 2017 at 6:07:01 pm BST, Dongjiu Geng <gengdongjiu@...wei.com> wrote:
> > For this matching, current code using the {I,D}FSC range to match
> > kvm_vcpu_trap_get_fault_type() return value, but
> > kvm_vcpu_trap_get_fault_type() only return the part {I,D}FSC instead
> > of whole, so fix this issue
> >
> > Value Type
> > 0x10 FSC_SEA
> > 0x14 FSC_SEA_TTW0 FSC_SEA_TTW1 FSC_SEA_TTW2 FSC_SEA_TTW3
> > 0x18 FSC_SECC
> > 0x1c FSC_SECC_TTW0 FSC_SECC_TTW1 FSC_SECC_TTW2 FSC_SECC_TTW3
> >
> > CC: James Morse <james.morse@....com>
> > CC: Tyler Baicar <tbaicar@...eaurora.org>
> > Signed-off-by: Dongjiu Geng <gengdongjiu@...wei.com>
> >
> > ---
> > As shown below code:
> >
> > The kvm_vcpu_trap_get_fault_type() only return {I,D}FSC bit[5]:bit[2],
> > not the whole {I,D}FSC, but FSC_SEA_TTWx and FSC_SECC_TTWx are the
> > whole {I,D}FSC value.
> >
> > static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu
> > *vcpu) {
> > return kvm_vcpu_get_hsr(vcpu) & ESR_ELx_FSC_TYPE; }
> >
> > static inline bool kvm_vcpu_dabt_isextabt(const struct kvm_vcpu *vcpu)
> > {
> > switch (kvm_vcpu_trap_get_fault_type(vcpu)) {
> > case FSC_SEA:
> > case FSC_SEA_TTW0:
> > case FSC_SEA_TTW1:
> > case FSC_SEA_TTW2:
> > case FSC_SEA_TTW3:
> > case FSC_SECC:
> > case FSC_SECC_TTW0:
> > case FSC_SECC_TTW1:
> > case FSC_SECC_TTW2:
> > case FSC_SECC_TTW3:
> > return true;
> > default:
> > return false;
> > }
> > }
> > ---
> > arch/arm/include/asm/kvm_arm.h | 10 ++--------
> > arch/arm/include/asm/kvm_emulate.h | 10 ++--------
> > arch/arm64/include/asm/kvm_arm.h | 10 ++--------
> > arch/arm64/include/asm/kvm_emulate.h | 10 ++--------
> > 4 files changed, 8 insertions(+), 32 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/kvm_arm.h
> > b/arch/arm/include/asm/kvm_arm.h index c878145..b4b155c 100644
> > --- a/arch/arm/include/asm/kvm_arm.h
> > +++ b/arch/arm/include/asm/kvm_arm.h
> > @@ -188,15 +188,9 @@
> > #define FSC_ACCESS (0x08)
> > #define FSC_PERM (0x0c)
> > #define FSC_SEA (0x10)
> > -#define FSC_SEA_TTW0 (0x14)
> > -#define FSC_SEA_TTW1 (0x15)
> > -#define FSC_SEA_TTW2 (0x16)
> > -#define FSC_SEA_TTW3 (0x17)
> > +#define FSC_SEA_TTW (0x14)
> > #define FSC_SECC (0x18)
> > -#define FSC_SECC_TTW0 (0x1c)
> > -#define FSC_SECC_TTW1 (0x1d)
> > -#define FSC_SECC_TTW2 (0x1e)
> > -#define FSC_SECC_TTW3 (0x1f)
> > +#define FSC_SECC_TTW (0x1c)
> >
> > /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > #define HPFAR_MASK (~0xf)
> > diff --git a/arch/arm/include/asm/kvm_emulate.h
> > b/arch/arm/include/asm/kvm_emulate.h
> > index 98089ff..aed8279 100644
> > --- a/arch/arm/include/asm/kvm_emulate.h
> > +++ b/arch/arm/include/asm/kvm_emulate.h
> > @@ -205,15 +205,9 @@ static inline bool kvm_vcpu_dabt_isextabt(struct
> > kvm_vcpu *vcpu) {
> > switch (kvm_vcpu_trap_get_fault_type(vcpu)) {
> > case FSC_SEA:
> > - case FSC_SEA_TTW0:
> > - case FSC_SEA_TTW1:
> > - case FSC_SEA_TTW2:
> > - case FSC_SEA_TTW3:
> > + case FSC_SEA_TTW:
> > case FSC_SECC:
> > - case FSC_SECC_TTW0:
> > - case FSC_SECC_TTW1:
> > - case FSC_SECC_TTW2:
> > - case FSC_SECC_TTW3:
> > + case FSC_SECC_TTW:
> > return true;
> > default:
> > return false;
> > diff --git a/arch/arm64/include/asm/kvm_arm.h
> > b/arch/arm64/include/asm/kvm_arm.h
> > index 61d694c..c0f1798 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -205,15 +205,9 @@
> > #define FSC_ACCESS ESR_ELx_FSC_ACCESS
> > #define FSC_PERM ESR_ELx_FSC_PERM
> > #define FSC_SEA ESR_ELx_FSC_EXTABT
> > -#define FSC_SEA_TTW0 (0x14)
> > -#define FSC_SEA_TTW1 (0x15)
> > -#define FSC_SEA_TTW2 (0x16)
> > -#define FSC_SEA_TTW3 (0x17)
> > +#define FSC_SEA_TTW (0x14)
> > #define FSC_SECC (0x18)
> > -#define FSC_SECC_TTW0 (0x1c)
> > -#define FSC_SECC_TTW1 (0x1d)
> > -#define FSC_SECC_TTW2 (0x1e)
> > -#define FSC_SECC_TTW3 (0x1f)
> > +#define FSC_SECC_TTW (0x1c)
> >
> > /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > #define HPFAR_MASK (~UL(0xf))
> > diff --git a/arch/arm64/include/asm/kvm_emulate.h
> > b/arch/arm64/include/asm/kvm_emulate.h
> > index e5df3fc..5cde311 100644
> > --- a/arch/arm64/include/asm/kvm_emulate.h
> > +++ b/arch/arm64/include/asm/kvm_emulate.h
> > @@ -239,15 +239,9 @@ static inline bool kvm_vcpu_dabt_isextabt(const
> > struct kvm_vcpu *vcpu) {
> > switch (kvm_vcpu_trap_get_fault_type(vcpu)) {
> > case FSC_SEA:
> > - case FSC_SEA_TTW0:
> > - case FSC_SEA_TTW1:
> > - case FSC_SEA_TTW2:
> > - case FSC_SEA_TTW3:
> > + case FSC_SEA_TTW:
> > case FSC_SECC:
> > - case FSC_SECC_TTW0:
> > - case FSC_SECC_TTW1:
> > - case FSC_SECC_TTW2:
> > - case FSC_SECC_TTW3:
> > + case FSC_SECC_TTW:
> > return true;
> > default:
> > return false;
>
> What is the bug you're fixing? From what I can tell, you're simply removing valuable symbols from the kernel, and inventing another one
> that doesn't exist in the architecture.
Marc, I mean the "kvm_vcpu_trap_get_fault_type(vcpu))" return the value of fault type which is not the syndrome value {D,I}FSC.
the case sentence matches the syndrome value instead of fault type. So the two are not match, I mean here has issue.
So the FSC_SECC_TTW1/ FSC_SECC_TTW2/ FSC_SECC_TTW3 and FSC_SECC_TTW1/ FSC_SECC_TTW2/ FSC_SECC_TTW3 are dead code.
Below value are syndrome {D,I}FSC value, which is not fault type.
FSC_SECC_TTW0:
FSC_SECC_TTW1
FSC_SECC_TTW2:
FSC_SECC_TTW3:
FSC_SEA_TTW1:
FSC_SEA_TTW2
FSC_SEA_TTW3:
kvm_vcpu_trap_get_fault_type() will clear the low two bits to zero. So I use FSC_SEA_TTW represent "Synchronous external abort on translation table walk"
As we can see the Translation fault type "FSC_FAULT", which does not define the "FSC_FAULT0" "FSC_FAULT1" "FSC_FAULT2" "FSC_FAULT3".
Because the fault type " FSC_FAULT " include the four cases.
static inline u8 kvm_vcpu_trap_get_fault(const struct kvm_vcpu *vcpu)
{
return kvm_vcpu_get_hsr(vcpu) & 0x3f;
}
static inline u8 kvm_vcpu_trap_get_fault_type(const struct kvm_vcpu *vcpu)
{
return kvm_vcpu_get_hsr(vcpu) & 0x3C;
}
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny.
Powered by blists - more mailing lists