netdev - Re: [PATCH net 3/5] netdevsim: avoid debugfs warning message when module is remove

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]

Message-ID: <CAMArcTUZ476vinLb2f+JfGB209=qYeSWFgAHgb4DJdt4o9OHKw@mail.gmail.com>
Date:   Thu, 16 Jan 2020 23:54:46 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     David Miller <davem@...emloft.net>, Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH net 3/5] netdevsim: avoid debugfs warning message when
 module is remove

On Sun, 12 Jan 2020 at 23:45, Jakub Kicinski <kuba@...nel.org> wrote:
>

Hi Jakub,
Thank you for catching the problem!

> On Sat, 11 Jan 2020 16:37:23 +0000, Taehee Yoo wrote:
> > When module is being removed, it couldn't be held by try_module_get().
> > debugfs's open function internally tries to hold file_operation->owner
> > if .owner is set.
> > If holding owner operation is failed, it prints a warning message.
>
> > [  412.227709][ T1720] debugfs file owner did not clean up at exit: ipsec
>
> > In order to avoid the warning message, this patch makes netdevsim module
> > does not set .owner. Unsetting .owner is safe because these are protected
> > by inode_lock().
>
> So inode_lock will protect from the code getting unloaded/disappearing?
> At a quick glance at debugs code it doesn't seem that inode_lock would
> do that. Could you explain a little more to a non-fs developer like
> myself? :)
>
> Alternatively should we perhaps hold a module reference for each device
> created and force user space to clean up the devices? That may require
> some fixes to the test which use netdevsim.
>

Sorry, I misunderstood the debugfs logic.
inode_lock() is called by debugfs_remove() and debugfs_create_file().
It doesn't protect read and write operations.

Currently, I have been taking look at debugfs_file_{get/put}() function,
which increases and decreases the reference counter.
In the __debugfs_file_removed(), this reference counter is used for
waiting read and write operations. Unfortunately, the
__debugfs_file_removed() isn't used because of "dentry->d_flags" value.
So, I'm looking for a way to use these functions.

Thanks a lot!
Taehee Yoo