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 <tadeusz.struk@linaro.org>
Change-Id: I08d9b545a2715423296bf4beb67bdbbed78d1be1
diff --git a/fs/incfs/data_mgmt.h b/fs/incfs/data_mgmt.h
index 84db038..2227913 100644
--- a/fs/incfs/data_mgmt.h
+++ b/fs/incfs/data_mgmt.h
@@ -131,8 +131,14 @@ 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;
struct dentry *mi_incomplete_dir;
+ /* For stacking mounts, if true, this indicates if the incomplete dir
+ * needs to be freed for this SB. Similar to mi_index_free */
+ bool mi_incomplete_free;
const struct cred *mi_owner;
diff --git a/fs/incfs/vfs.c b/fs/incfs/vfs.c
index d2f65d2..9cdf7b9 100644
--- a/fs/incfs/vfs.c
+++ b/fs/incfs/vfs.c
@@ -436,7 +436,8 @@ static int incfs_init_dentry(struct dentry *dentry, struct path *path)
}
static struct dentry *open_or_create_special_dir(struct dentry *backing_dir,
- const char *name)
+ const char *name,
+ bool *created)
{
struct dentry *index_dentry;
struct inode *backing_inode = d_inode(backing_dir);
@@ -449,6 +450,7 @@ static struct dentry *open_or_create_special_dir(struct dentry *backing_dir,
return index_dentry;
} else if (d_really_is_positive(index_dentry)) {
/* Index already exists. */
+ *created = false;
return index_dentry;
}
@@ -468,6 +470,7 @@ static struct dentry *open_or_create_special_dir(struct dentry *backing_dir,
return ERR_PTR(-EINVAL);
}
+ *created = true;
return index_dentry;
}
@@ -1746,6 +1749,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))
@@ -1762,17 +1766,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_free_opts;
}
error = kern_path(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY,
@@ -1781,57 +1791,66 @@ 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_free_opts;
}
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;
}
+ sb->s_fs_info = mi;
+ mi->mi_backing_dir_path = backing_dir_path;
index_dir = open_or_create_special_dir(backing_dir_path.dentry,
- INCFS_INDEX_NAME);
+ INCFS_INDEX_NAME, &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);
/* No need to null index_dir since we don't put it */
- goto err;
+ goto err_put_path;
}
+
mi->mi_index_dir = index_dir;
+ mi->mi_index_free = dir_created;
incomplete_dir = open_or_create_special_dir(backing_dir_path.dentry,
- INCFS_INCOMPLETE_NAME);
+ INCFS_INCOMPLETE_NAME,
+ &dir_created);
if (IS_ERR_OR_NULL(incomplete_dir)) {
error = PTR_ERR(incomplete_dir);
pr_err("incfs: Can't find or create .incomplete dir in %s\n",
dev_name);
/* No need to null incomplete_dir since we don't put it */
- goto err;
+ goto err_put_path;
}
mi->mi_incomplete_dir = incomplete_dir;
+ mi->mi_incomplete_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;
@@ -1839,12 +1858,14 @@ struct dentry *incfs_mount_fs(struct file_system_type *type, int flags,
pr_debug("incfs: mount\n");
free_options(&options);
return dget(sb->s_root);
-err:
- sb->s_fs_info = NULL;
+
+err_put_path:
path_put(&backing_dir_path);
- incfs_free_mount_info(mi);
- deactivate_locked_super(sb);
+err_free_opts:
free_options(&options);
+err_deactivate:
+ deactivate_locked_super(sb);
+ pr_err("incfs: mount failed %d\n", error);
return ERR_PTR(error);
}
@@ -1879,10 +1900,26 @@ 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");
- generic_shutdown_super(sb);
- incfs_free_mount_info(mi);
+
+ 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);
+
+ if (mi->mi_incomplete_dir && mi->mi_incomplete_free)
+ vfs_rmdir(dinode, mi->mi_incomplete_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)
