From a15a5f5953c89b427132a6db7ab09302a5ac4796 Mon Sep 17 00:00:00 2001 From: ChenXiaoSong Date: Sun, 25 Jun 2023 08:20:11 +0000 Subject: [PATCH] hmdfs: fix possible use-after-free in hmdfs lookup() DCACHE_PAR_LOOKUP flag of dentry in hmdfs lookup() may be cleard after d_add()/d_splice_alias(), dereference dentry after d_add()/d_splice_alias() is not allowed. Signed-off-by: ChenXiaoSong --- fs/hmdfs/authority/authentication.c | 5 ++++- fs/hmdfs/authority/authentication.h | 2 ++ fs/hmdfs/inode_cloud.c | 7 ++++--- fs/hmdfs/inode_cloud_merge.c | 17 ++++++++--------- fs/hmdfs/inode_merge.c | 17 ++++++++--------- fs/hmdfs/inode_remote.c | 14 +++++++++----- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/fs/hmdfs/authority/authentication.c b/fs/hmdfs/authority/authentication.c index 076e738c92e7..cfbb4f19bf0f 100644 --- a/fs/hmdfs/authority/authentication.c +++ b/fs/hmdfs/authority/authentication.c @@ -358,14 +358,17 @@ void check_and_fixup_ownership(struct inode *parent_inode, struct inode *child) } void check_and_fixup_ownership_remote(struct inode *dir, + struct inode *dinode, struct dentry *dentry) { struct hmdfs_inode_info *hii = hmdfs_i(dir); - struct inode *dinode = d_inode(dentry); struct hmdfs_inode_info *dinfo = hmdfs_i(dinode); __u16 level = hmdfs_perm_get_next_level(hii->perm); __u16 perm = 0; + if (IS_ERR_OR_NULL(dinode)) + return; + hmdfs_debug("level:0x%X", level); switch (level) { case HMDFS_PERM_MNT: diff --git a/fs/hmdfs/authority/authentication.h b/fs/hmdfs/authority/authentication.h index a36636a78286..d66c19898aba 100644 --- a/fs/hmdfs/authority/authentication.h +++ b/fs/hmdfs/authority/authentication.h @@ -261,6 +261,7 @@ int hmdfs_override_dir_id_fs(struct cache_fs_override *or, __u16 *perm); void hmdfs_revert_dir_id_fs(struct cache_fs_override *or); void check_and_fixup_ownership_remote(struct inode *dir, + struct inode *dinode, struct dentry *dentry); extern int get_bid(const char *bname); extern int __init hmdfs_init_configfs(void); @@ -322,6 +323,7 @@ void hmdfs_revert_creds(const struct cred *old) static inline void check_and_fixup_ownership_remote(struct inode *dir, + struct inode *inode, struct dentry *dentry) { } diff --git a/fs/hmdfs/inode_cloud.c b/fs/hmdfs/inode_cloud.c index a34f598914e2..868f918345a7 100644 --- a/fs/hmdfs/inode_cloud.c +++ b/fs/hmdfs/inode_cloud.c @@ -302,12 +302,13 @@ static struct dentry *hmdfs_lookup_cloud_dentry(struct inode *parent_inode, if (in_share_dir(child_dentry)) gdi->file_type = HM_SHARE; inode = fill_inode_cloud(sb, lookup_result, parent_inode); + + check_and_fixup_ownership_remote(parent_inode, + inode, + child_dentry); ret = d_splice_alias(inode, child_dentry); if (!IS_ERR_OR_NULL(ret)) child_dentry = ret; - if (!IS_ERR(ret)) - check_and_fixup_ownership_remote(parent_inode, - child_dentry); } else { ret = ERR_PTR(-ENOENT); } diff --git a/fs/hmdfs/inode_cloud_merge.c b/fs/hmdfs/inode_cloud_merge.c index 07fbedb32882..029c6305e75e 100644 --- a/fs/hmdfs/inode_cloud_merge.c +++ b/fs/hmdfs/inode_cloud_merge.c @@ -293,6 +293,14 @@ struct dentry *hmdfs_lookup_cloud_merge(struct inode *parent_inode, child_inode = fill_inode_merge(parent_inode->i_sb, parent_inode, child_dentry, NULL); + info = hmdfs_i(child_inode); + if (info->inode_type == HMDFS_LAYER_FIRST_MERGE) + hmdfs_root_inode_perm_init(child_inode); + else + check_and_fixup_ownership_remote(parent_inode, + child_inode, + child_dentry); + ret_dentry = d_splice_alias(child_inode, child_dentry); if (IS_ERR(ret_dentry)) { clear_comrades(child_dentry); @@ -303,13 +311,6 @@ struct dentry *hmdfs_lookup_cloud_merge(struct inode *parent_inode, update_dm(ret_dentry, child_dentry); child_dentry = ret_dentry; } - info = hmdfs_i(child_inode); - if (info->inode_type == HMDFS_LAYER_FIRST_MERGE_CLOUD) - hmdfs_root_inode_perm_init(child_inode); - else - check_and_fixup_ownership_remote(parent_inode, - child_dentry); - goto out; } @@ -317,8 +318,6 @@ struct dentry *hmdfs_lookup_cloud_merge(struct inode *parent_inode, err = 0; out: - hmdfs_trace_merge(trace_hmdfs_lookup_merge_end, parent_inode, - child_dentry, err); return err ? ERR_PTR(err) : ret_dentry; } diff --git a/fs/hmdfs/inode_merge.c b/fs/hmdfs/inode_merge.c index 0ef8defc2b86..59f9f4b5e97b 100644 --- a/fs/hmdfs/inode_merge.c +++ b/fs/hmdfs/inode_merge.c @@ -739,6 +739,14 @@ struct dentry *hmdfs_lookup_merge(struct inode *parent_inode, child_inode = fill_inode_merge(parent_inode->i_sb, parent_inode, child_dentry, NULL); + info = hmdfs_i(child_inode); + if (info->inode_type == HMDFS_LAYER_FIRST_MERGE) + hmdfs_root_inode_perm_init(child_inode); + else + check_and_fixup_ownership_remote(parent_inode, + child_inode, + child_dentry); + ret_dentry = d_splice_alias(child_inode, child_dentry); if (IS_ERR(ret_dentry)) { clear_comrades(child_dentry); @@ -748,13 +756,6 @@ struct dentry *hmdfs_lookup_merge(struct inode *parent_inode, if (ret_dentry) { child_dentry = ret_dentry; } - info = hmdfs_i(child_inode); - if (info->inode_type == HMDFS_LAYER_FIRST_MERGE) - hmdfs_root_inode_perm_init(child_inode); - else - check_and_fixup_ownership_remote(parent_inode, - child_dentry); - goto out; } @@ -762,8 +763,6 @@ struct dentry *hmdfs_lookup_merge(struct inode *parent_inode, err = 0; out: - hmdfs_trace_merge(trace_hmdfs_lookup_merge_end, parent_inode, - child_dentry, err); return err ? ERR_PTR(err) : ret_dentry; } diff --git a/fs/hmdfs/inode_remote.c b/fs/hmdfs/inode_remote.c index f906b2410407..5cc991e191d3 100644 --- a/fs/hmdfs/inode_remote.c +++ b/fs/hmdfs/inode_remote.c @@ -449,12 +449,12 @@ static struct dentry *hmdfs_lookup_remote_dentry(struct inode *parent_inode, if (in_share_dir(child_dentry)) gdi->file_type = HM_SHARE; inode = fill_inode_remote(sb, con, lookup_result, parent_inode); + check_and_fixup_ownership_remote(parent_inode, + inode, + child_dentry); ret = d_splice_alias(inode, child_dentry); if (!IS_ERR_OR_NULL(ret)) child_dentry = ret; - if (!IS_ERR(ret)) - check_and_fixup_ownership_remote(parent_inode, - child_dentry); } else { ret = ERR_PTR(-ENOENT); } @@ -559,11 +559,13 @@ int hmdfs_mkdir_remote_dentry(struct hmdfs_peer *conn, struct dentry *dentry, } if (mkdir_ret) { inode = fill_inode_remote(sb, conn, mkdir_ret, parent_inode); + check_and_fixup_ownership_remote(parent_inode, + inode, + dentry); if (!IS_ERR(inode)) d_add(dentry, inode); else err = PTR_ERR(inode); - check_and_fixup_ownership_remote(parent_inode, dentry); } else { err = -ENOENT; } @@ -629,11 +631,13 @@ int hmdfs_create_remote_dentry(struct hmdfs_peer *conn, struct dentry *dentry, } if (create_ret) { inode = fill_inode_remote(sb, conn, create_ret, parent_inode); + check_and_fixup_ownership_remote(parent_inode, + inode, + dentry); if (!IS_ERR(inode)) d_add(dentry, inode); else err = PTR_ERR(inode); - check_and_fixup_ownership_remote(parent_inode, dentry); } else { err = -ENOENT; hmdfs_err("get remote inode info failed err = %d", err); -- Gitee