Commit 2a348dff authored by Al Viro's avatar Al Viro Committed by Kleber Sacilotto de Souza

configfs: fix a deadlock in configfs_symlink()

BugLink: https://bugs.launchpad.net/bugs/1852335

commit 351e5d86 upstream.

Configfs abuses symlink(2).  Unlike the normal filesystems, it
wants the target resolved at symlink(2) time, like link(2) would've
done.  The problem is that ->symlink() is called with the parent
directory locked exclusive, so resolving the target inside the
->symlink() is easily deadlocked.

Short of really ugly games in sys_symlink() itself, all we can
do is to unlock the parent before resolving the target and
relock it after.  However, that invalidates the checks done
by the caller of ->symlink(), so we have to
	* check that dentry is still where it used to be
(it couldn't have been moved, but it could've been unhashed)
	* recheck that it's still negative (somebody else
might've successfully created a symlink with the same name
while we were looking the target up)
	* recheck the permissions on the parent directory.

Cc: stable@vger.kernel.org
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarConnor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent 977814bb
......@@ -157,11 +157,42 @@ int configfs_symlink(struct inode *dir, struct dentry *dentry, const char *symna
!type->ct_item_ops->allow_link)
goto out_put;
/*
* This is really sick. What they wanted was a hybrid of
* link(2) and symlink(2) - they wanted the target resolved
* at syscall time (as link(2) would've done), be a directory
* (which link(2) would've refused to do) *AND* be a deep
* fucking magic, making the target busy from rmdir POV.
* symlink(2) is nothing of that sort, and the locking it
* gets matches the normal symlink(2) semantics. Without
* attempts to resolve the target (which might very well
* not even exist yet) done prior to locking the parent
* directory. This perversion, OTOH, needs to resolve
* the target, which would lead to obvious deadlocks if
* attempted with any directories locked.
*
* Unfortunately, that garbage is userland ABI and we should've
* said "no" back in 2005. Too late now, so we get to
* play very ugly games with locking.
*
* Try *ANYTHING* of that sort in new code, and you will
* really regret it. Just ask yourself - what could a BOFH
* do to me and do I want to find it out first-hand?
*
* AV, a thoroughly annoyed bastard.
*/
inode_unlock(dir);
ret = get_target(symname, &path, &target_item, dentry->d_sb);
inode_lock(dir);
if (ret)
goto out_put;
ret = type->ct_item_ops->allow_link(parent_item, target_item);
if (dentry->d_inode || d_unhashed(dentry))
ret = -EEXIST;
else
ret = inode_permission(dir, MAY_WRITE | MAY_EXEC);
if (!ret)
ret = type->ct_item_ops->allow_link(parent_item, target_item);
if (!ret) {
mutex_lock(&configfs_symlink_mutex);
ret = create_link(parent_item, target_item, dentry);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment