Skip to content

Commit

Permalink
ANDROID: incremental-fs: limit mount stack depth
Browse files Browse the repository at this point in the history
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 <tadeusz.struk@linaro.org>
Change-Id: I08d9b545a2715423296bf4beb67bdbbed78d1be1
  • Loading branch information
tstruk committed Apr 8, 2022
1 parent b155e04 commit 2c142e4
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 18 deletions.
3 changes: 3 additions & 0 deletions fs/incfs/data_mgmt.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
67 changes: 49 additions & 18 deletions fs/incfs/vfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,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;
Expand All @@ -771,6 +772,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;
}

Expand All @@ -789,6 +791,7 @@ static struct dentry *open_or_create_index_dir(struct dentry *backing_dir)
return ERR_PTR(-EINVAL);
}

*created = true;
return index_dentry;
}

Expand Down Expand Up @@ -2177,6 +2180,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))
Expand All @@ -2193,17 +2197,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,
Expand All @@ -2212,55 +2222,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;

mi = incfs_alloc_mount_info(sb, &options, &backing_dir_path);
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;
mi->mi_index_free = dir_created;

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;
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);
}

Expand All @@ -2286,10 +2305,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);

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)
Expand Down

0 comments on commit 2c142e4

Please sign in to comment.