Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

configfs: fix a deadlock in configfs_symlink()

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: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christoph Hellwig <hch@lst.de>

authored by

Al Viro and committed by
Christoph Hellwig
351e5d86 f74c2bb9

+32 -1
+32 -1
fs/configfs/symlink.c
··· 143 143 !type->ct_item_ops->allow_link) 144 144 goto out_put; 145 145 146 + /* 147 + * This is really sick. What they wanted was a hybrid of 148 + * link(2) and symlink(2) - they wanted the target resolved 149 + * at syscall time (as link(2) would've done), be a directory 150 + * (which link(2) would've refused to do) *AND* be a deep 151 + * fucking magic, making the target busy from rmdir POV. 152 + * symlink(2) is nothing of that sort, and the locking it 153 + * gets matches the normal symlink(2) semantics. Without 154 + * attempts to resolve the target (which might very well 155 + * not even exist yet) done prior to locking the parent 156 + * directory. This perversion, OTOH, needs to resolve 157 + * the target, which would lead to obvious deadlocks if 158 + * attempted with any directories locked. 159 + * 160 + * Unfortunately, that garbage is userland ABI and we should've 161 + * said "no" back in 2005. Too late now, so we get to 162 + * play very ugly games with locking. 163 + * 164 + * Try *ANYTHING* of that sort in new code, and you will 165 + * really regret it. Just ask yourself - what could a BOFH 166 + * do to me and do I want to find it out first-hand? 167 + * 168 + * AV, a thoroughly annoyed bastard. 169 + */ 170 + inode_unlock(dir); 146 171 ret = get_target(symname, &path, &target_item, dentry->d_sb); 172 + inode_lock(dir); 147 173 if (ret) 148 174 goto out_put; 149 175 150 - ret = type->ct_item_ops->allow_link(parent_item, target_item); 176 + if (dentry->d_inode || d_unhashed(dentry)) 177 + ret = -EEXIST; 178 + else 179 + ret = inode_permission(dir, MAY_WRITE | MAY_EXEC); 180 + if (!ret) 181 + ret = type->ct_item_ops->allow_link(parent_item, target_item); 151 182 if (!ret) { 152 183 mutex_lock(&configfs_symlink_mutex); 153 184 ret = create_link(parent_item, target_item, dentry);