| Message ID | 20241024083350.2828920-1-cuigaosheng1@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] KVM: arm64: Fix possible null-ptr-deref in idregs_debug_show | expand |
On Thu, 24 Oct 2024 09:33:50 +0100, Gaosheng Cui <cuigaosheng1@huawei.com> wrote: > > The idregs_debug_show() maybe return nullptr, we need to check desc > before dereference it to avoid possible null pointer dereferences. > > Fixes: 410db103f6eb ("KVM: arm64: Make idregs debugfs iterator search sysreg table directly") > Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> > --- > arch/arm64/kvm/sys_regs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index ff8c4e1b847e..b1c59773b9c0 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -4135,7 +4135,7 @@ static int idregs_debug_show(struct seq_file *s, void *v) > > desc = idregs_debug_find(kvm, kvm->arch.idreg_debugfs_iter); > > - if (!desc->name) > + if (!desc || !desc->name) > return 0; > > seq_printf(s, "%20s:\t%016llx\n", Can you show a case where this happens in practice? The check for NULL is already in idregs_debug_next(), and I don't see how this can actually be triggered. Thanks, M.
On 2024/10/24 17:29, Marc Zyngier wrote: > On Thu, 24 Oct 2024 09:33:50 +0100, > Gaosheng Cui <cuigaosheng1@huawei.com> wrote: >> The idregs_debug_show() maybe return nullptr, we need to check desc >> before dereference it to avoid possible null pointer dereferences. >> >> Fixes: 410db103f6eb ("KVM: arm64: Make idregs debugfs iterator search sysreg table directly") >> Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> >> --- >> arch/arm64/kvm/sys_regs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c >> index ff8c4e1b847e..b1c59773b9c0 100644 >> --- a/arch/arm64/kvm/sys_regs.c >> +++ b/arch/arm64/kvm/sys_regs.c >> @@ -4135,7 +4135,7 @@ static int idregs_debug_show(struct seq_file *s, void *v) >> >> desc = idregs_debug_find(kvm, kvm->arch.idreg_debugfs_iter); >> >> - if (!desc->name) >> + if (!desc || !desc->name) >> return 0; >> >> seq_printf(s, "%20s:\t%016llx\n", > Can you show a case where this happens in practice? > > The check for NULL is already in idregs_debug_next(), and I don't see > how this can actually be triggered. I haven't found a scenario that can trigger this problem，this patch was discovered during code review，I will try to prove it. Thanks for your work. > > Thanks, > > M. >
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index ff8c4e1b847e..b1c59773b9c0 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -4135,7 +4135,7 @@ static int idregs_debug_show(struct seq_file *s, void *v) desc = idregs_debug_find(kvm, kvm->arch.idreg_debugfs_iter); - if (!desc->name) + if (!desc || !desc->name) return 0; seq_printf(s, "%20s:\t%016llx\n",
The idregs_debug_show() maybe return nullptr, we need to check desc before dereference it to avoid possible null pointer dereferences. Fixes: 410db103f6eb ("KVM: arm64: Make idregs debugfs iterator search sysreg table directly") Signed-off-by: Gaosheng Cui <cuigaosheng1@huawei.com> --- arch/arm64/kvm/sys_regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)