From: Zizhi Wo <wozizhi@huaweicloud.com> hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/ICFXTV CVE: NA -------------------------------- The current calling process may trigger a potential ABBA deadlock. The calling stack is as follows: do_mkdirat user_path_create filename_create mnt_want_write --lock a inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT) --lock b vfs_write cachefiles_daemon_write cachefiles_daemon_cull cachefiles_cull cachefiles_check_active inode_lock_nested(d_inode(dir), I_MUTEX_PARENT) --lock b cachefiles_remove_object_xattr mnt_want_write --lock a vfs_removexattr This is because there is a problem with the lock order. mnt_want_write() should be called first, and then inode_lock_nested(). Fix the lock order. And delete the redundant code in cachefiles_check_old_object_xattr(), because this part of the process cannot be triggered. Fixes: 2a0beff2d223 ("cachefiles: Fix non-taking of sb_writers around set/removexattr") Signed-off-by: Zizhi Wo <wozizhi@huaweicloud.com> --- fs/cachefiles/namei.c | 9 ++++++++- fs/cachefiles/xattr.c | 20 ++++---------------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/fs/cachefiles/namei.c b/fs/cachefiles/namei.c index 6eeef666c609..6fc8c504ae1b 100644 --- a/fs/cachefiles/namei.c +++ b/fs/cachefiles/namei.c @@ -990,9 +990,15 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir, _enter(",%pd/,%s", dir, filename); + ret = mnt_want_write(cache->mnt); + if (ret < 0) + return ret; + victim = cachefiles_check_active(cache, dir, filename); - if (IS_ERR(victim)) + if (IS_ERR(victim)) { + mnt_drop_write(cache->mnt); return PTR_ERR(victim); + } _debug("victim -> %p %s", victim, d_backing_inode(victim) ? "positive" : "negative"); @@ -1003,6 +1009,7 @@ int cachefiles_cull(struct cachefiles_cache *cache, struct dentry *dir, _debug("victim is cullable"); ret = cachefiles_remove_object_xattr(cache, victim); + mnt_drop_write(cache->mnt); if (ret < 0) goto error_unlock; diff --git a/fs/cachefiles/xattr.c b/fs/cachefiles/xattr.c index bac55fc7359e..646a8002d0cf 100644 --- a/fs/cachefiles/xattr.c +++ b/fs/cachefiles/xattr.c @@ -243,7 +243,6 @@ int cachefiles_check_old_object_xattr(struct cachefiles_object *object, struct cachefiles_xattr *auxdata) { struct cachefiles_xattr *auxbuf; - struct cachefiles_cache *cache; unsigned int len = sizeof(struct cachefiles_xattr) + 512; struct dentry *dentry = object->dentry; int ret; @@ -301,17 +300,10 @@ int cachefiles_check_old_object_xattr(struct cachefiles_object *object, BUG(); } - cache = container_of(object->fscache.cache, - struct cachefiles_cache, cache); - /* update the current label */ - ret = mnt_want_write(cache->mnt); - if (ret == 0) { - ret = vfs_setxattr(dentry, cachefiles_xattr_cache, - &auxdata->type, auxdata->len, - XATTR_REPLACE); - mnt_drop_write(cache->mnt); - } + ret = vfs_setxattr(dentry, cachefiles_xattr_cache, + &auxdata->type, auxdata->len, + XATTR_REPLACE); if (ret < 0) { cachefiles_io_error_obj(object, "Can't update xattr on %lu" @@ -393,11 +385,7 @@ int cachefiles_remove_object_xattr(struct cachefiles_cache *cache, { int ret; - ret = mnt_want_write(cache->mnt); - if (ret == 0) { - ret = vfs_removexattr(dentry, cachefiles_xattr_cache); - mnt_drop_write(cache->mnt); - } + ret = vfs_removexattr(dentry, cachefiles_xattr_cache); if (ret < 0) { if (ret == -ENOENT || ret == -ENODATA) ret = 0; -- 2.39.2