Skip to content

Commit a535116

Browse files
committed
ovl: make use of ->layers safe in rcu pathwalk
ovl_permission() accesses ->layers[...].mnt; we can't have ->layers freed without an RCU delay on fs shutdown. Fortunately, kern_unmount_array() that is used to drop those mounts does include an RCU delay, so freeing is delayed; unfortunately, the array passed to kern_unmount_array() is formed by mangling ->layers contents and that happens without any delays. The ->layers[...].name string entries are used to store the strings to display in "lowerdir=..." by ovl_show_options(). Those entries are not accessed in RCU walk. Move the name strings into a separate array ofs->config.lowerdirs and reuse the ofs->config.lowerdirs array as the temporary mount array to pass to kern_unmount_array(). Reported-by: Al Viro <[email protected]> Link: https://lore.kernel.org/r/20231002023711.GP3389589@ZenIV/ Acked-by: Miklos Szeredi <[email protected]> Signed-off-by: Amir Goldstein <[email protected]>
1 parent c54719c commit a535116

File tree

3 files changed

+21
-24
lines changed

3 files changed

+21
-24
lines changed

fs/overlayfs/ovl_entry.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
struct ovl_config {
99
char *upperdir;
1010
char *workdir;
11+
char **lowerdirs;
1112
bool default_permissions;
1213
int redirect_mode;
1314
int verity_mode;
@@ -39,17 +40,8 @@ struct ovl_layer {
3940
int idx;
4041
/* One fsid per unique underlying sb (upper fsid == 0) */
4142
int fsid;
42-
char *name;
4343
};
4444

45-
/*
46-
* ovl_free_fs() relies on @mnt being the first member when unmounting
47-
* the private mounts created for each layer. Let's check both the
48-
* offset and type.
49-
*/
50-
static_assert(offsetof(struct ovl_layer, mnt) == 0);
51-
static_assert(__same_type(typeof_member(struct ovl_layer, mnt), struct vfsmount *));
52-
5345
struct ovl_path {
5446
const struct ovl_layer *layer;
5547
struct dentry *dentry;

fs/overlayfs/params.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -752,19 +752,20 @@ void ovl_free_fs(struct ovl_fs *ofs)
752752
if (ofs->upperdir_locked)
753753
ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
754754

755-
/* Hack! Reuse ofs->layers as a vfsmount array before freeing it */
756-
mounts = (struct vfsmount **) ofs->layers;
755+
/* Reuse ofs->config.lowerdirs as a vfsmount array before freeing it */
756+
mounts = (struct vfsmount **) ofs->config.lowerdirs;
757757
for (i = 0; i < ofs->numlayer; i++) {
758758
iput(ofs->layers[i].trap);
759+
kfree(ofs->config.lowerdirs[i]);
759760
mounts[i] = ofs->layers[i].mnt;
760-
kfree(ofs->layers[i].name);
761761
}
762762
kern_unmount_array(mounts, ofs->numlayer);
763763
kfree(ofs->layers);
764764
for (i = 0; i < ofs->numfs; i++)
765765
free_anon_bdev(ofs->fs[i].pseudo_dev);
766766
kfree(ofs->fs);
767767

768+
kfree(ofs->config.lowerdirs);
768769
kfree(ofs->config.upperdir);
769770
kfree(ofs->config.workdir);
770771
if (ofs->creator_cred)
@@ -949,16 +950,16 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry)
949950
struct super_block *sb = dentry->d_sb;
950951
struct ovl_fs *ofs = OVL_FS(sb);
951952
size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer;
952-
const struct ovl_layer *data_layers = &ofs->layers[nr_merged_lower];
953+
char **lowerdatadirs = &ofs->config.lowerdirs[nr_merged_lower];
953954

954-
/* ofs->layers[0] is the upper layer */
955-
seq_printf(m, ",lowerdir=%s", ofs->layers[1].name);
955+
/* lowerdirs[] starts from offset 1 */
956+
seq_printf(m, ",lowerdir=%s", ofs->config.lowerdirs[1]);
956957
/* dump regular lower layers */
957958
for (nr = 2; nr < nr_merged_lower; nr++)
958-
seq_printf(m, ":%s", ofs->layers[nr].name);
959+
seq_printf(m, ":%s", ofs->config.lowerdirs[nr]);
959960
/* dump data lower layers */
960961
for (nr = 0; nr < ofs->numdatalayer; nr++)
961-
seq_printf(m, "::%s", data_layers[nr].name);
962+
seq_printf(m, "::%s", lowerdatadirs[nr]);
962963
if (ofs->config.upperdir) {
963964
seq_show_option(m, "upperdir", ofs->config.upperdir);
964965
seq_show_option(m, "workdir", ofs->config.workdir);

fs/overlayfs/super.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -572,11 +572,6 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
572572
upper_layer->idx = 0;
573573
upper_layer->fsid = 0;
574574

575-
err = -ENOMEM;
576-
upper_layer->name = kstrdup(ofs->config.upperdir, GFP_KERNEL);
577-
if (!upper_layer->name)
578-
goto out;
579-
580575
/*
581576
* Inherit SB_NOSEC flag from upperdir.
582577
*
@@ -1125,7 +1120,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
11251120
layers[ofs->numlayer].idx = ofs->numlayer;
11261121
layers[ofs->numlayer].fsid = fsid;
11271122
layers[ofs->numlayer].fs = &ofs->fs[fsid];
1128-
layers[ofs->numlayer].name = l->name;
1123+
/* Store for printing lowerdir=... in ovl_show_options() */
1124+
ofs->config.lowerdirs[ofs->numlayer] = l->name;
11291125
l->name = NULL;
11301126
ofs->numlayer++;
11311127
ofs->fs[fsid].is_lower = true;
@@ -1370,8 +1366,16 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
13701366
if (!layers)
13711367
goto out_err;
13721368

1369+
ofs->config.lowerdirs = kcalloc(ctx->nr + 1, sizeof(char *), GFP_KERNEL);
1370+
if (!ofs->config.lowerdirs) {
1371+
kfree(layers);
1372+
goto out_err;
1373+
}
13731374
ofs->layers = layers;
1374-
/* Layer 0 is reserved for upper even if there's no upper */
1375+
/*
1376+
* Layer 0 is reserved for upper even if there's no upper.
1377+
* For consistency, config.lowerdirs[0] is NULL.
1378+
*/
13751379
ofs->numlayer = 1;
13761380

13771381
sb->s_stack_depth = 0;

0 commit comments

Comments
 (0)