linux-kernel - Re: [PATCH] kvm: no need to check return value of debugfs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 22 Jan 2019 21:48:28 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Christian Borntraeger <borntraeger@...ibm.com>
Cc:     linux-kernel@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        kvm@...r.kernel.org
Subject: Re: [PATCH] kvm: no need to check return value of debugfs_create
 functions

On Tue, Jan 22, 2019 at 09:39:24PM +0100, Christian Borntraeger wrote:
> 
> 
> On 22.01.2019 16:21, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> > 
> > Cc: Paolo Bonzini <pbonzini@...hat.com>
> > Cc: "Radim Krčmář" <rkrcmar@...hat.com>
> > Cc: kvm@...r.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > ---
> >  virt/kvm/kvm_main.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 5ecea812cb6a..4f96450ecdfc 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2528,9 +2528,7 @@ static int kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> > 
> >  	snprintf(dir_name, sizeof(dir_name), "vcpu%d", vcpu->vcpu_id);
> >  	vcpu->debugfs_dentry = debugfs_create_dir(dir_name,
> > -								vcpu->kvm->debugfs_dentry);
> > -	if (!vcpu->debugfs_dentry)
> > -		return -ENOMEM;
> > +						  vcpu->kvm->debugfs_dentry);
> > 
> >  	ret = kvm_arch_create_vcpu_debugfs(vcpu);
> >  	if (ret < 0) {
> > 
> 
> 
> The interesting part of these debugfs entries is that they export an interface that is used
> by the kvm_stat tool. (and all distributions that I checked have debugfs enabled).
> 
> I think it is pretty unlikely that things will fail, but the question is: do we want to reject
> VM creation if that VM cannot be observed by instrumentation or not? No idea.

No you should not as any other part of the kernel can randomly create
the same debugfs files and keep your code from being able to create
them :)

> This also brings the question: shall we move these counters out of debugfs into something else?

If you have code that relies on debugfs, yes, you need to move that out
of debugfs because more and more systems are trying to disable it due to
the obvious problems with it (i.e. leaking tons of debugging
information).

debugfs is for DEBUG information, not for "statistics about how my VM is
working".  That sounds like something you need to rely on, so debugfs is
not the place for it.

thanks,

greg k-h