From a894c2e4c8b5f4b77cc79c4f1afe889bd5350ac8 Mon Sep 17 00:00:00 2001 From: Tadeusz Struk Date: Tue, 8 Mar 2022 17:20:15 -0800 Subject: [PATCH] ANDROID: incremental-fs: limit mount stack depth Syzbot recently found a number of issues related to incremental-fs (see bug numbers below). All have to do with the fact that incr-fs allows mounts of the same source and target multiple times. This is a design decision and the user space component "Data Loader" expects this to work for app re-install use case. The mounting depth needs to be controlled, however, and only allowed to be two levels deep. In case of more than two mount attempts the driver needs to return an error. In case of the issues listed below the common pattern is that the reproducer calls: mount("./file0", "./file0", "incremental-fs", 0, NULL) many times and then invokes a file operation like chmod, setxattr, or open on the ./file0. This causes a recursive call for all the mounted instances, which eventually causes a stack overflow and a kernel crash: BUG: stack guard page was hit at ffffc90000c0fff8 kernel stack overflow (double-fault): 0000 [#1] PREEMPT SMP KASAN This change also cleans up the mount error path to properly clean allocated resources and call deactivate_locked_super(), which causes the incfs_kill_sb() to be called, where the sb is freed. Bug: 211066171 Bug: 213140206 Bug: 213215835 Bug: 211914587 Bug: 211213635 Bug: 213137376 Bug: 211161296 Signed-off-by: Tadeusz Struk Change-Id: I08d9b545a2715423296bf4beb67bdbbed78d1be1 --- fs/incfs/data_mgmt.h | 3 ++ fs/incfs/vfs.c | 70 +++++++++++++++++++++++++++++++------------- 2 files changed, 53 insertions(+), 20 deletions(-) diff --git a/fs/incfs/data_mgmt.h b/fs/incfs/data_mgmt.h index 2726867835a8..a98858afb138 100644 --- a/fs/incfs/data_mgmt.h +++ b/fs/incfs/data_mgmt.h @@ -109,6 +109,9 @@ struct mount_info { struct path mi_backing_dir_path; struct dentry *mi_index_dir; + /* For stacking mounts, if true, this indicates if the index dir needs + * to be freed for this SB otherwise it was created by lower level SB */ + bool mi_index_free; const struct cred *mi_owner; diff --git a/fs/incfs/vfs.c b/fs/incfs/vfs.c index b849568ae9b8..f696cf651afb 100644 --- a/fs/incfs/vfs.c +++ b/fs/incfs/vfs.c @@ -755,7 +755,8 @@ static struct dentry *incfs_lookup_dentry(struct dentry *parent, return result; } -static struct dentry *open_or_create_index_dir(struct dentry *backing_dir) +static struct dentry *open_or_create_index_dir(struct dentry *backing_dir, + bool *created) { static const char name[] = ".index"; struct dentry *index_dentry; @@ -769,6 +770,7 @@ static struct dentry *open_or_create_index_dir(struct dentry *backing_dir) return index_dentry; } else if (d_really_is_positive(index_dentry)) { /* Index already exists. */ + *created = false; return index_dentry; } @@ -787,6 +789,7 @@ static struct dentry *open_or_create_index_dir(struct dentry *backing_dir) return ERR_PTR(-EINVAL); } + *created = true; return index_dentry; } @@ -2175,6 +2178,7 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, struct super_block *src_fs_sb = NULL; struct inode *root_inode = NULL; struct super_block *sb = sget(type, NULL, set_anon_super, flags, NULL); + bool dir_created = false; int error = 0; if (IS_ERR(sb)) @@ -2191,17 +2195,23 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, BUILD_BUG_ON(PAGE_SIZE != INCFS_DATA_FILE_BLOCK_SIZE); + if (!dev_name) { + pr_err("incfs: Backing dir is not set, filesystem can't be mounted.\n"); + error = -ENOENT; + goto err_deactivate; + } + error = parse_options(&options, (char *)data); if (error != 0) { pr_err("incfs: Options parsing error. %d\n", error); - goto err; + goto err_deactivate; } sb->s_bdi->ra_pages = options.readahead_pages; if (!dev_name) { pr_err("incfs: Backing dir is not set, filesystem can't be mounted.\n"); error = -ENOENT; - goto err; + goto err_deactivate; } error = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, @@ -2210,55 +2220,64 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, !d_really_is_positive(backing_dir_path.dentry)) { pr_err("incfs: Error accessing: %s.\n", dev_name); - goto err; + goto err_deactivate; } src_fs_sb = backing_dir_path.dentry->d_sb; sb->s_maxbytes = src_fs_sb->s_maxbytes; + sb->s_stack_depth = src_fs_sb->s_stack_depth + 1; + + if (sb->s_stack_depth > FILESYSTEM_MAX_STACK_DEPTH) { + error = -EINVAL; + goto err_put_path; + } mi = incfs_alloc_mount_info(sb, &options, &backing_dir_path); - if (IS_ERR_OR_NULL(mi)) { error = PTR_ERR(mi); pr_err("incfs: Error allocating mount info. %d\n", error); - mi = NULL; - goto err; + goto err_put_path; } - index_dir = open_or_create_index_dir(backing_dir_path.dentry); + sb->s_fs_info = mi; + mi->mi_backing_dir_path = backing_dir_path; + index_dir = open_or_create_index_dir(backing_dir_path.dentry, + &dir_created); if (IS_ERR_OR_NULL(index_dir)) { error = PTR_ERR(index_dir); pr_err("incfs: Can't find or create .index dir in %s\n", dev_name); - goto err; + goto err_put_path; } - mi->mi_index_dir = index_dir; - sb->s_fs_info = mi; + mi->mi_index_dir = index_dir; + mi->mi_index_free = dir_created; + root_inode = fetch_regular_inode(sb, backing_dir_path.dentry); if (IS_ERR(root_inode)) { error = PTR_ERR(root_inode); - goto err; + goto err_put_path; } sb->s_root = d_make_root(root_inode); if (!sb->s_root) { error = -ENOMEM; - goto err; + goto err_put_path; } error = incfs_init_dentry(sb->s_root, &backing_dir_path); if (error) - goto err; + goto err_put_path; path_put(&backing_dir_path); sb->s_flags |= SB_ACTIVE; pr_debug("incfs: mount\n"); return dget(sb->s_root); -err: - sb->s_fs_info = NULL; + +err_put_path: path_put(&backing_dir_path); - incfs_free_mount_info(mi); +err_deactivate: deactivate_locked_super(sb); + pr_err("incfs: mount failed %d\n", error); return ERR_PTR(error); } @@ -2284,11 +2303,22 @@ static int incfs_remount_fs(struct super_block *sb, int *flags, char *data) void incfs_kill_sb(struct super_block *sb) { struct mount_info *mi = sb->s_fs_info; + struct inode *dinode = NULL; pr_debug("incfs: unmount\n"); - incfs_free_mount_info(mi); - generic_shutdown_super(sb); - sb->s_fs_info = NULL; + + if (mi) { + if (mi->mi_backing_dir_path.dentry) + dinode = d_inode(mi->mi_backing_dir_path.dentry); + + if (dinode) { + if (mi->mi_index_dir && mi->mi_index_free) + vfs_rmdir(dinode, mi->mi_index_dir); + } + incfs_free_mount_info(mi); + sb->s_fs_info = NULL; + } + kill_anon_super(sb); } static int show_options(struct seq_file *m, struct dentry *root)