Commit ba65dc5e authored by Al Viro's avatar Al Viro
Browse files

much milder d_walk() race 



d_walk() relies upon the tree not getting rearranged under it without
rename_lock being touched.  And we do grab rename_lock around the
places that change the tree topology.  Unfortunately, branch reordering
is just as bad from d_walk() POV and we have two places that do it
without touching rename_lock - one in handling of cursors (for ramfs-style
directories) and another in autofs.  autofs one is a separate story; this
commit deals with the cursors.
	* mark cursor dentries explicitly at allocation time
	* make __dentry_kill() leave ->d_child.next pointing to the next
non-cursor sibling, making sure that it won't be moved around unnoticed
before the parent is relocked on ascend-to-parent path in d_walk().
	* make d_walk() skip cursors explicitly; strictly speaking it's
not necessary (all callbacks we pass to d_walk() are no-ops on cursors),
but it makes analysis easier.

Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 1607f09c

fs/dcache.c

+52 −6
Original line number Diff line number Diff line
@@ -507,6 +507,44 @@ void d_drop(struct dentry *dentry) 
}

EXPORT_SYMBOL(d_drop);



static inline void dentry_unlist(struct dentry *dentry, struct dentry *parent)

{

	struct dentry *next;

	/*

	 * Inform d_walk() and shrink_dentry_list() that we are no longer

	 * attached to the dentry tree

	 */

	dentry->d_flags |= DCACHE_DENTRY_KILLED;

	if (unlikely(list_empty(&dentry->d_child)))

		return;

	__list_del_entry(&dentry->d_child);

	/*

	 * Cursors can move around the list of children.  While we'd been

	 * a normal list member, it didn't matter - ->d_child.next would've

	 * been updated.  However, from now on it won't be and for the

	 * things like d_walk() it might end up with a nasty surprise.

	 * Normally d_walk() doesn't care about cursors moving around -

	 * ->d_lock on parent prevents that and since a cursor has no children

	 * of its own, we get through it without ever unlocking the parent.

	 * There is one exception, though - if we ascend from a child that

	 * gets killed as soon as we unlock it, the next sibling is found

	 * using the value left in its ->d_child.next.  And if _that_

	 * pointed to a cursor, and cursor got moved (e.g. by lseek())

	 * before d_walk() regains parent->d_lock, we'll end up skipping

	 * everything the cursor had been moved past.

	 *

	 * Solution: make sure that the pointer left behind in ->d_child.next

	 * points to something that won't be moving around.  I.e. skip the

	 * cursors.

	 */

	while (dentry->d_child.next != &parent->d_subdirs) {

		next = list_entry(dentry->d_child.next, struct dentry, d_child);

		if (likely(!(next->d_flags & DCACHE_DENTRY_CURSOR)))

			break;

		dentry->d_child.next = next->d_child.next;

	}

}



static void __dentry_kill(struct dentry *dentry)

{

	struct dentry *parent = NULL;
@@ -532,12 +570,7 @@ static void __dentry_kill(struct dentry *dentry) 
	}

	/* if it was on the hash then remove it */

	__d_drop(dentry);

	__list_del_entry(&dentry->d_child);

	/*

	 * Inform d_walk() that we are no longer attached to the

	 * dentry tree

	 */

	dentry->d_flags |= DCACHE_DENTRY_KILLED;

	dentry_unlist(dentry, parent);

	if (parent)

		spin_unlock(&parent->d_lock);

	dentry_iput(dentry);
@@ -1203,6 +1236,9 @@ resume: 
		struct dentry *dentry = list_entry(tmp, struct dentry, d_child);

		next = tmp->next;



		if (unlikely(dentry->d_flags & DCACHE_DENTRY_CURSOR))

			continue;



		spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);



		ret = enter(data, dentry);
@@ -1651,6 +1687,16 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name) 
}

EXPORT_SYMBOL(d_alloc);



struct dentry *d_alloc_cursor(struct dentry * parent)

{

	struct dentry *dentry = __d_alloc(parent->d_sb, NULL);

	if (dentry) {

		dentry->d_flags |= DCACHE_RCUACCESS | DCACHE_DENTRY_CURSOR;

		dentry->d_parent = dget(parent);

	}

	return dentry;

}



/**

 * d_alloc_pseudo - allocate a dentry (for lookup-less filesystems)

 * @sb: the superblock

fs/internal.h

+1 −0
Original line number Diff line number Diff line
@@ -130,6 +130,7 @@ extern int invalidate_inodes(struct super_block *, bool); 
extern struct dentry *__d_alloc(struct super_block *, const struct qstr *);

extern int d_set_mounted(struct dentry *dentry);

extern long prune_dcache_sb(struct super_block *sb, struct shrink_control *sc);

extern struct dentry *d_alloc_cursor(struct dentry *);



/*

 * read_write.c

fs/libfs.c

+1 −3
Original line number Diff line number Diff line
@@ -71,9 +71,7 @@ EXPORT_SYMBOL(simple_lookup); 


int dcache_dir_open(struct inode *inode, struct file *file)

{

	static struct qstr cursor_name = QSTR_INIT(".", 1);



	file->private_data = d_alloc(file->f_path.dentry, &cursor_name);

	file->private_data = d_alloc_cursor(file->f_path.dentry);



	return file->private_data ? 0 : -ENOMEM;

}

include/linux/dcache.h

+1 −0
Original line number Diff line number Diff line
@@ -212,6 +212,7 @@ struct dentry_operations { 
#define DCACHE_OP_REAL			0x08000000



#define DCACHE_PAR_LOOKUP		0x10000000 /* being looked up (with parent locked shared) */

#define DCACHE_DENTRY_CURSOR		0x20000000



extern seqlock_t rename_lock;

CRA Git | Maintained and supported by SUSTech CRA and CCSE