From fd4c6594f5ce87eb3f6d53bd73eb14689305fdf1 Mon Sep 17 00:00:00 2001 From: Tadeusz Struk Date: Wed, 12 Jan 2022 13:52:50 -0800 Subject: [PATCH] ANDROID: incremental-fs: fix mount_fs issue 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. The correct behavior for a file system is to allow only one such mount, and then every subsequent attempt should fail with a -EBUSY error code. 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 The reason why many mounts with the same source and target are possible is because the incfs_mount_fs() as it is allocates a new super_block for every call, regardless of whether a given mount already exists or not. This happens every time the sget() function is called with a test param equal to NULL. The correct behavior for an FS mount implementation is to call appropriate mount vfs call for it's type, i.e. mount_bdev() for a block device backed FS, mount_single() for a pseudo file system, like sysfs that is mounted in a single, well know location, or mount_nodev() for other special purpose FS like overlayfs. In case of incremental-fs the open coded mount logic doesn't check for abusive mount attempts such as overlays. To fix this issue the logic needs to be changed to pass a proper test function to sget() call, which then checks if a super_block for a mount instance has already been allocated and also allows the VFS to properly verify invalid mount attempts. Bug: 211066171 Bug: 213140206 Bug: 213215835 Bug: 211914587 Bug: 211213635 Bug: 213137376 Bug: 211161296 Signed-off-by: Tadeusz Struk Change-Id: I66cfc3f1b5aaffb32b0845b2dad3ff26fe952e27 --- fs/incfs/data_mgmt.c | 1 + fs/incfs/vfs.c | 58 ++++++++++++++++++++++++++++++++------------ fs/incfs/vfs.h | 1 - 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/fs/incfs/data_mgmt.c b/fs/incfs/data_mgmt.c index fbab68a280d5..a383c5b5ad7f 100644 --- a/fs/incfs/data_mgmt.c +++ b/fs/incfs/data_mgmt.c @@ -175,6 +175,7 @@ void incfs_free_mount_info(struct mount_info *mi) kfree(mi->pseudo_file_xattr[i].data); kfree(mi->mi_per_uid_read_timeouts); incfs_free_sysfs_node(mi->mi_sysfs_node); + kfree(mi->mi_options.sysfs_name); kfree(mi); } diff --git a/fs/incfs/vfs.c b/fs/incfs/vfs.c index ea7866fbfd6e..35ac6e3daf8e 100644 --- a/fs/incfs/vfs.c +++ b/fs/incfs/vfs.c @@ -393,7 +393,7 @@ static int iterate_incfs_dir(struct file *file, struct dir_context *ctx) struct mount_info *mi = get_mount_info(file_superblock(file)); bool root; - if (!dir) { + if (!dir || !mi) { error = -EBADF; goto out; } @@ -1336,6 +1336,9 @@ static int dir_rename(struct inode *old_dir, struct dentry *old_dentry, struct dentry *trap; int error = 0; + if (!mi) + return -EBADF; + error = mutex_lock_interruptible(&mi->mi_dir_struct_mutex); if (error) return error; @@ -1664,6 +1667,9 @@ static ssize_t incfs_getxattr(struct dentry *d, const char *name, size_t stored_size; int i; + if (!mi) + return -EBADF; + if (di && di->backing_path.dentry) return vfs_getxattr(di->backing_path.dentry, name, value, size); @@ -1698,6 +1704,9 @@ static ssize_t incfs_setxattr(struct dentry *d, const char *name, size_t *stored_size; int i; + if (!mi) + return -EBADF; + if (di && di->backing_path.dentry) return vfs_setxattr(di->backing_path.dentry, name, value, size, flags); @@ -1736,6 +1745,11 @@ static ssize_t incfs_listxattr(struct dentry *d, char *list, size_t size) return vfs_listxattr(di->backing_path.dentry, list, size); } +static int incfs_test_super(struct super_block *s, void *p) +{ + return s->s_fs_info != NULL; +} + struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, const char *dev_name, void *data) { @@ -1746,7 +1760,8 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, struct dentry *incomplete_dir = NULL; struct super_block *src_fs_sb = NULL; struct inode *root_inode = NULL; - struct super_block *sb = sget(type, NULL, set_anon_super, flags, NULL); + struct super_block *sb = sget(type, incfs_test_super, set_anon_super, + flags, NULL); int error = 0; if (IS_ERR(sb)) @@ -1787,13 +1802,18 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, src_fs_sb = backing_dir_path.dentry->d_sb; sb->s_maxbytes = src_fs_sb->s_maxbytes; - mi = incfs_alloc_mount_info(sb, &options, &backing_dir_path); + if (!sb->s_fs_info) { + 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; + if (IS_ERR_OR_NULL(mi)) { + error = PTR_ERR(mi); + pr_err("incfs: Error allocating mount info. %d\n", error); + mi = NULL; + goto err; + } + sb->s_fs_info = mi; + } else { + mi = sb->s_fs_info; } index_dir = open_or_create_special_dir(backing_dir_path.dentry, @@ -1818,21 +1838,22 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags, } mi->mi_incomplete_dir = incomplete_dir; - sb->s_fs_info = mi; root_inode = fetch_regular_inode(sb, backing_dir_path.dentry); if (IS_ERR(root_inode)) { error = PTR_ERR(root_inode); goto err; } - sb->s_root = d_make_root(root_inode); if (!sb->s_root) { - error = -ENOMEM; - goto err; + sb->s_root = d_make_root(root_inode); + if (!sb->s_root) { + error = -ENOMEM; + goto err; + } + error = incfs_init_dentry(sb->s_root, &backing_dir_path); + if (error) + goto err; } - error = incfs_init_dentry(sb->s_root, &backing_dir_path); - if (error) - goto err; path_put(&backing_dir_path); sb->s_flags |= SB_ACTIVE; @@ -1854,6 +1875,9 @@ static int incfs_remount_fs(struct super_block *sb, int *flags, char *data) struct mount_info *mi = get_mount_info(sb); int err = 0; + if (!mi) + return err; + sync_filesystem(sb); err = parse_options(&options, (char *)data); if (err) @@ -1883,12 +1907,16 @@ void incfs_kill_sb(struct super_block *sb) pr_debug("incfs: unmount\n"); generic_shutdown_super(sb); incfs_free_mount_info(mi); + sb->s_fs_info = NULL; } static int show_options(struct seq_file *m, struct dentry *root) { struct mount_info *mi = get_mount_info(root->d_sb); + if (!mi) + return -EBADF; + seq_printf(m, ",read_timeout_ms=%u", mi->mi_options.read_timeout_ms); seq_printf(m, ",readahead=%u", mi->mi_options.readahead_pages); if (mi->mi_options.read_log_pages != 0) { diff --git a/fs/incfs/vfs.h b/fs/incfs/vfs.h index 79fdf243733d..8876e63a8b0f 100644 --- a/fs/incfs/vfs.h +++ b/fs/incfs/vfs.h @@ -19,7 +19,6 @@ static inline struct mount_info *get_mount_info(struct super_block *sb) { struct mount_info *result = sb->s_fs_info; - WARN_ON(!result); return result; }