Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dfef6dcd authored Mar 08, 2011 by Al Viro

unfuck proc_sysctl ->d_compare()



a) struct inode is not going to be freed under ->d_compare();
however, the thing PROC_I(inode)->sysctl points to just might.
Fortunately, it's enough to make freeing that sucker delayed,
provided that we don't step on its ->unregistering, clear
the pointer to it in PROC_I(inode) before dropping the reference
and check if it's NULL in ->d_compare().

b) I'm not sure that we *can* walk into NULL inode here (we recheck
dentry->seq between verifying that it's still hashed / fetching
dentry->d_inode and passing it to ->d_compare() and there's no
negative hashed dentries in /proc/sys/*), but if we can walk into
that, we really should not have ->d_compare() return 0 on it!
Said that, I really suspect that this check can be simply killed.
Nick?

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

parent 1858efd4

fs/proc/inode.c

+6 −2

Original line number	Original line	Diff line number	Diff line
	@@ -27,6 +27,7 @@
	static void proc_evict_inode(struct inode *inode)		static void proc_evict_inode(struct inode *inode)
	{		{
	struct proc_dir_entry *de;		struct proc_dir_entry *de;
			struct ctl_table_header *head;

	truncate_inode_pages(&inode->i_data, 0);		truncate_inode_pages(&inode->i_data, 0);
	end_writeback(inode);		end_writeback(inode);
	@@ -38,8 +39,11 @@ static void proc_evict_inode(struct inode *inode)
	de = PROC_I(inode)->pde;		de = PROC_I(inode)->pde;
	if (de)		if (de)
	pde_put(de);		pde_put(de);
	if (PROC_I(inode)->sysctl)		head = PROC_I(inode)->sysctl;
	sysctl_head_put(PROC_I(inode)->sysctl);		if (head) {
			rcu_assign_pointer(PROC_I(inode)->sysctl, NULL);
			sysctl_head_put(head);
			}
	}		}

	struct vfsmount *proc_mnt;		struct vfsmount *proc_mnt;

fs/proc/proc_sysctl.c

+5 −2

Original line number	Original line	Diff line number	Diff line
	@@ -408,15 +408,18 @@ static int proc_sys_compare(const struct dentry *parent,
	const struct dentry dentry, const struct inode inode,		const struct dentry dentry, const struct inode inode,
	unsigned int len, const char str, const struct qstr name)		unsigned int len, const char str, const struct qstr name)
	{		{
			struct ctl_table_header *head;
	/* Although proc doesn't have negative dentries, rcu-walk means		/* Although proc doesn't have negative dentries, rcu-walk means
	* that inode here can be NULL */		* that inode here can be NULL */
			/* AV: can it, indeed? */
	if (!inode)		if (!inode)
	return 0;		return 1;
	if (name->len != len)		if (name->len != len)
	return 1;		return 1;
	if (memcmp(name->name, str, len))		if (memcmp(name->name, str, len))
	return 1;		return 1;
	return !sysctl_is_seen(PROC_I(inode)->sysctl);		head = rcu_dereference(PROC_I(inode)->sysctl);
			return !head \|\| !sysctl_is_seen(head);
	}		}

	static const struct dentry_operations proc_sys_dentry_operations = {		static const struct dentry_operations proc_sys_dentry_operations = {

include/linux/sysctl.h

+10 −4

Original line number	Original line	Diff line number	Diff line
	@@ -25,6 +25,7 @@
	#include <linux/kernel.h>		#include <linux/kernel.h>
	#include <linux/types.h>		#include <linux/types.h>
	#include <linux/compiler.h>		#include <linux/compiler.h>
			#include <linux/rcupdate.h>

	struct completion;		struct completion;

	@@ -1037,10 +1038,15 @@ struct ctl_table_root {
	struct ctl_table trees. */		struct ctl_table trees. */
	struct ctl_table_header		struct ctl_table_header
	{		{
			union {
			struct {
	struct ctl_table *ctl_table;		struct ctl_table *ctl_table;
	struct list_head ctl_entry;		struct list_head ctl_entry;
	int used;		int used;
	int count;		int count;
			};
			struct rcu_head rcu;
			};
	struct completion *unregistering;		struct completion *unregistering;
	struct ctl_table *ctl_table_arg;		struct ctl_table *ctl_table_arg;
	struct ctl_table_root *root;		struct ctl_table_root *root;

kernel/sysctl.c

+10 −5

Original line number	Original line	Diff line number	Diff line
	@@ -194,9 +194,9 @@ static int sysrq_sysctl_handler(ctl_table *table, int write,
	static struct ctl_table root_table[];		static struct ctl_table root_table[];
	static struct ctl_table_root sysctl_table_root;		static struct ctl_table_root sysctl_table_root;
	static struct ctl_table_header root_table_header = {		static struct ctl_table_header root_table_header = {
	.count = 1,		{{.count = 1,
	.ctl_table = root_table,		.ctl_table = root_table,
	.ctl_entry = LIST_HEAD_INIT(sysctl_table_root.default_set.list),		.ctl_entry = LIST_HEAD_INIT(sysctl_table_root.default_set.list),}},
	.root = &sysctl_table_root,		.root = &sysctl_table_root,
	.set = &sysctl_table_root.default_set,		.set = &sysctl_table_root.default_set,
	};		};
	@@ -1567,11 +1567,16 @@ void sysctl_head_get(struct ctl_table_header *head)
	spin_unlock(&sysctl_lock);		spin_unlock(&sysctl_lock);
	}		}

			static void free_head(struct rcu_head *rcu)
			{
			kfree(container_of(rcu, struct ctl_table_header, rcu));
			}

	void sysctl_head_put(struct ctl_table_header *head)		void sysctl_head_put(struct ctl_table_header *head)
	{		{
	spin_lock(&sysctl_lock);		spin_lock(&sysctl_lock);
	if (!--head->count)		if (!--head->count)
	kfree(head);		call_rcu(&head->rcu, free_head);
	spin_unlock(&sysctl_lock);		spin_unlock(&sysctl_lock);
	}		}

	@@ -1948,10 +1953,10 @@ void unregister_sysctl_table(struct ctl_table_header * header)
	start_unregistering(header);		start_unregistering(header);
	if (!--header->parent->count) {		if (!--header->parent->count) {
	WARN_ON(1);		WARN_ON(1);
	kfree(header->parent);		call_rcu(&header->parent->rcu, free_head);
	}		}
	if (!--header->count)		if (!--header->count)
	kfree(header);		call_rcu(&header->rcu, free_head);
	spin_unlock(&sysctl_lock);		spin_unlock(&sysctl_lock);
	}		}