linux-kernel - Re: [PATCH] kvm: x86: Use DEFINE_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>] [day] [month] [year] [list]

Date:   Wed, 18 Sep 2019 14:03:52 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     wang.yi59@....com.cn
Cc:     pbonzini@...hat.com, rkrcmar@...hat.com, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, hpa@...or.com, x86@...nel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        xue.zhihong@....com.cn, up2wing@...il.com, wang.liang82@....com.cn
Subject: Re: [PATCH] kvm: x86: Use DEFINE_DEBUGFS_ATTRIBUTE for debugfs files

On Wed, Sep 18, 2019 at 08:19:44AM +0800, wang.yi59@....com.cn wrote:
> Hi Paolo,
> 
> > On 22/07/19 09:33, Yi Wang wrote:
> > > We got these coccinelle warning:
> > > ./arch/x86/kvm/debugfs.c:23:0-23: WARNING: vcpu_timer_advance_ns_fops
> > > should be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:32:0-23: WARNING: vcpu_tsc_offset_fops should
> > > be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:41:0-23: WARNING: vcpu_tsc_scaling_fops should
> > > be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > > ./arch/x86/kvm/debugfs.c:49:0-23: WARNING: vcpu_tsc_scaling_frac_fops
> > > should be defined with DEFINE_DEBUGFS_ATTRIBUTE
> > >
> > > Use DEFINE_DEBUGFS_ATTRIBUTE() rather than DEFINE_SIMPLE_ATTRIBUTE()
> > > to fix this.
> > >
> > > Signed-off-by: Yi Wang <wang.yi59@....com.cn>
> >
> > It sucks though that you have to use a function with "unsafe" in the name.
> 
> Yes, it does, but I found some patches in the git log:
> https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?qt=grep&q=DEFINE_DEBUGFS_ATTRIBUTE+
> 
> And, do you think the function name "debugfs_create_file_unsafe" is not proper?

Only if you _KNOW_ you are creating/removing these files in a way that
is safe is it ok to use these calls.  Hint, what is your locking
strategy for when these files are removed?

Is that the case here?  If not, please stick with what is there today,
as we know it works, and it is "safe" to do so.

thanks,

greg k-h