linux-kernel - Re: debugfs_remove() vs. anything that is dynamic

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:	Fri, 4 Apr 2008 13:20:06 -0700
From:	Greg KH <greg@...ah.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	Linux Kernel list <linux-kernel@...r.kernel.org>
Subject: Re: debugfs_remove() vs. anything that is dynamic

On Fri, Apr 04, 2008 at 05:41:24PM +0200, Johannes Berg wrote:
> 
> > If you worry about this type of interaction, use debugfs_create_file,
> > which takes a fileops, and set your module owner in there so that the
> > reference count will not allow your module from being removed.
> >
> > Also remember, you have to be root to unload modules, so if you are
> > doing that, and you have debugfs files open, you should know better :)
> 
> That really was just an example, we have per-wireless-device debugfs
> code users can easily trigger a debugfs_remove() by unplugging a usb
> netdevice for example. Also, that means that anything that is dynamic
> would have the lifetime rules imposed by debugfs which is rather
> awkward.

Then use the debugfs_create_file() function instead of the individual
variable ones if this is the issue.  You can easily wrap them up much
like the debugfs core does with the common wrappers to allow for the
proper module lifetime rules to be followed.

> The current code for simple_attr_open copies the in i_private pointer:
> 
> attr->data = inode->i_private;
> 
> if, instead, it would keep a reference to that, like
> 
> attr->dataptr = &???inode->i_private;
> 
> we could NULL out that pointer on debugfs_remove() and have
> simple_attr_read() just return -ENOENT.

Patches are always gladly accepted :)

> However, if nobody else is concerned about this, I'll just remove all
> the wireless debugfs code instead, I just don't want to allow crashing
> it that way.

I think you should be able to handle this in your debugfs calls, or if
needed, we can easily add a new parameter to them to handle the module
ownership if you are concerned about them.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/