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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 4 Apr 2008 08:34:02 -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 01:56:26AM +0200, Johannes Berg wrote:
> Consider the following trivial module:
> 
> --- %< ---
> #include <linux/module.h>
> #include <linux/debugfs.h>
> 
> static struct dentry *f;
> static u32 tmp;
> 
> int __init mod_enter(void)
> {
> 	f = debugfs_create_u32("tmp-test", 0666, NULL, &tmp);
> 
> 	return 0;
> }
> 
> void __exit mod_leave(void)
> {
> 	debugfs_remove(f);
> }
> 
> module_init(mod_enter);
> module_exit(mod_leave);
> MODULE_LICENSE("GPL");
> --- >% ---
> 
> How do I make that safe?
> 
> 
> FWIW, the problem is:
> 
> thread 1			thread 2
>  fd = open("tmp-test")
> 
>  sleep(30);			rmmod test-module
> 
>  read(fd, buf, 100);
> 
> --> accesses now invalid memory because debugfs doesn't actually stop
> you from accessing "&tmp" after debugfs_remove(). [yes, I actually
> tested a variation of this where I dynamically allocated the 'tmp'
> variable, I got the slab poison in my test program]
> 
> 
> Personally, I tend to think this makes debugfs rather unusable in
> modules and with anything that is dynamically allocated [1]. AFAICT
> sysfs avoids this by having object lifetime imposed by sysfs, but
> debugfs doesn't work that way.
> 
> What am I missing?

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 :)

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/