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 <tadeusz.struk@linaro.org>
Change-Id: I66cfc3f1b5aaffb32b0845b2dad3ff26fe952e27
This commit is contained in:
Tadeusz Struk 2022-01-12 13:52:50 -08:00
parent a512242e66
commit fd4c6594f5
3 changed files with 44 additions and 16 deletions

View File

@ -175,6 +175,7 @@ void incfs_free_mount_info(struct mount_info *mi)
kfree(mi->pseudo_file_xattr[i].data); kfree(mi->pseudo_file_xattr[i].data);
kfree(mi->mi_per_uid_read_timeouts); kfree(mi->mi_per_uid_read_timeouts);
incfs_free_sysfs_node(mi->mi_sysfs_node); incfs_free_sysfs_node(mi->mi_sysfs_node);
kfree(mi->mi_options.sysfs_name);
kfree(mi); kfree(mi);
} }

View File

@ -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)); struct mount_info *mi = get_mount_info(file_superblock(file));
bool root; bool root;
if (!dir) { if (!dir || !mi) {
error = -EBADF; error = -EBADF;
goto out; goto out;
} }
@ -1336,6 +1336,9 @@ static int dir_rename(struct inode *old_dir, struct dentry *old_dentry,
struct dentry *trap; struct dentry *trap;
int error = 0; int error = 0;
if (!mi)
return -EBADF;
error = mutex_lock_interruptible(&mi->mi_dir_struct_mutex); error = mutex_lock_interruptible(&mi->mi_dir_struct_mutex);
if (error) if (error)
return error; return error;
@ -1664,6 +1667,9 @@ static ssize_t incfs_getxattr(struct dentry *d, const char *name,
size_t stored_size; size_t stored_size;
int i; int i;
if (!mi)
return -EBADF;
if (di && di->backing_path.dentry) if (di && di->backing_path.dentry)
return vfs_getxattr(di->backing_path.dentry, name, value, size); 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; size_t *stored_size;
int i; int i;
if (!mi)
return -EBADF;
if (di && di->backing_path.dentry) if (di && di->backing_path.dentry)
return vfs_setxattr(di->backing_path.dentry, name, value, size, return vfs_setxattr(di->backing_path.dentry, name, value, size,
flags); 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); 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, struct dentry *incfs_mount_fs(struct file_system_type *type, int flags,
const char *dev_name, void *data) 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 dentry *incomplete_dir = NULL;
struct super_block *src_fs_sb = NULL; struct super_block *src_fs_sb = NULL;
struct inode *root_inode = 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; int error = 0;
if (IS_ERR(sb)) 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; src_fs_sb = backing_dir_path.dentry->d_sb;
sb->s_maxbytes = src_fs_sb->s_maxbytes; 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)) { if (IS_ERR_OR_NULL(mi)) {
error = PTR_ERR(mi); error = PTR_ERR(mi);
pr_err("incfs: Error allocating mount info. %d\n", error); pr_err("incfs: Error allocating mount info. %d\n", error);
mi = NULL; mi = NULL;
goto err; goto err;
}
sb->s_fs_info = mi;
} else {
mi = sb->s_fs_info;
} }
index_dir = open_or_create_special_dir(backing_dir_path.dentry, 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; mi->mi_incomplete_dir = incomplete_dir;
sb->s_fs_info = mi;
root_inode = fetch_regular_inode(sb, backing_dir_path.dentry); root_inode = fetch_regular_inode(sb, backing_dir_path.dentry);
if (IS_ERR(root_inode)) { if (IS_ERR(root_inode)) {
error = PTR_ERR(root_inode); error = PTR_ERR(root_inode);
goto err; goto err;
} }
sb->s_root = d_make_root(root_inode);
if (!sb->s_root) { if (!sb->s_root) {
error = -ENOMEM; sb->s_root = d_make_root(root_inode);
goto err; 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); path_put(&backing_dir_path);
sb->s_flags |= SB_ACTIVE; 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); struct mount_info *mi = get_mount_info(sb);
int err = 0; int err = 0;
if (!mi)
return err;
sync_filesystem(sb); sync_filesystem(sb);
err = parse_options(&options, (char *)data); err = parse_options(&options, (char *)data);
if (err) if (err)
@ -1883,12 +1907,16 @@ void incfs_kill_sb(struct super_block *sb)
pr_debug("incfs: unmount\n"); pr_debug("incfs: unmount\n");
generic_shutdown_super(sb); generic_shutdown_super(sb);
incfs_free_mount_info(mi); incfs_free_mount_info(mi);
sb->s_fs_info = NULL;
} }
static int show_options(struct seq_file *m, struct dentry *root) static int show_options(struct seq_file *m, struct dentry *root)
{ {
struct mount_info *mi = get_mount_info(root->d_sb); 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, ",read_timeout_ms=%u", mi->mi_options.read_timeout_ms);
seq_printf(m, ",readahead=%u", mi->mi_options.readahead_pages); seq_printf(m, ",readahead=%u", mi->mi_options.readahead_pages);
if (mi->mi_options.read_log_pages != 0) { if (mi->mi_options.read_log_pages != 0) {

View File

@ -19,7 +19,6 @@ static inline struct mount_info *get_mount_info(struct super_block *sb)
{ {
struct mount_info *result = sb->s_fs_info; struct mount_info *result = sb->s_fs_info;
WARN_ON(!result);
return result; return result;
} }