Commit 403688e0 authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'ovl-fixes-6.6-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs

Pull overlayfs fixes from Amir Goldstein:

 - Fix for file reference leak regression

 - Fix for NULL pointer deref regression

 - Fixes for RCU-walk race regressions:

   Two of the fixes were taken from Al's RCU pathwalk race fixes series
   with his consent [1].

   Note that unlike most of Al's series, these two patches are not about
   racing with ->kill_sb() and they are also very recent regressions
   from v6.5, so I think it's worth getting them into v6.5.y.

   There is also a fix for an RCU pathwalk race with ->kill_sb(), which
   may have been solved in vfs generic code as you suggested, but it
   also rids overlayfs from a nasty hack, so I think it's worth anyway.

Link: https://lore.kernel.org/linux-fsdevel/20231003204749.GA800259@ZenIV/ [1]

* tag 'ovl-fixes-6.6-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/overlayfs/vfs:
  ovl: fix NULL pointer defer when encoding non-decodable lower fid
  ovl: make use of ->layers safe in rcu pathwalk
  ovl: fetch inode once in ovl_dentry_revalidate_common()
  ovl: move freeing ovl_entry past rcu delay
  ovl: fix file reference leak when submitting aio
parents 3006adf3 c7242a45
...@@ -188,7 +188,7 @@ static int ovl_check_encode_origin(struct dentry *dentry) ...@@ -188,7 +188,7 @@ static int ovl_check_encode_origin(struct dentry *dentry)
/* Lower file handle for non-upper non-decodable */ /* Lower file handle for non-upper non-decodable */
if (!ovl_dentry_upper(dentry) && !decodable) if (!ovl_dentry_upper(dentry) && !decodable)
return 0; return 1;
/* Upper file handle for pure upper */ /* Upper file handle for pure upper */
if (!ovl_dentry_lower(dentry)) if (!ovl_dentry_lower(dentry))
......
...@@ -341,7 +341,6 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter) ...@@ -341,7 +341,6 @@ static ssize_t ovl_read_iter(struct kiocb *iocb, struct iov_iter *iter)
if (!aio_req) if (!aio_req)
goto out; goto out;
real.flags = 0;
aio_req->orig_iocb = iocb; aio_req->orig_iocb = iocb;
kiocb_clone(&aio_req->iocb, iocb, get_file(real.file)); kiocb_clone(&aio_req->iocb, iocb, get_file(real.file));
aio_req->iocb.ki_complete = ovl_aio_rw_complete; aio_req->iocb.ki_complete = ovl_aio_rw_complete;
...@@ -413,7 +412,6 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter) ...@@ -413,7 +412,6 @@ static ssize_t ovl_write_iter(struct kiocb *iocb, struct iov_iter *iter)
if (!aio_req) if (!aio_req)
goto out; goto out;
real.flags = 0;
aio_req->orig_iocb = iocb; aio_req->orig_iocb = iocb;
kiocb_clone(&aio_req->iocb, iocb, get_file(real.file)); kiocb_clone(&aio_req->iocb, iocb, get_file(real.file));
aio_req->iocb.ki_flags = ifl; aio_req->iocb.ki_flags = ifl;
......
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
struct ovl_config { struct ovl_config {
char *upperdir; char *upperdir;
char *workdir; char *workdir;
char **lowerdirs;
bool default_permissions; bool default_permissions;
int redirect_mode; int redirect_mode;
int verity_mode; int verity_mode;
...@@ -39,17 +40,8 @@ struct ovl_layer { ...@@ -39,17 +40,8 @@ struct ovl_layer {
int idx; int idx;
/* One fsid per unique underlying sb (upper fsid == 0) */ /* One fsid per unique underlying sb (upper fsid == 0) */
int fsid; int fsid;
char *name;
}; };
/*
* ovl_free_fs() relies on @mnt being the first member when unmounting
* the private mounts created for each layer. Let's check both the
* offset and type.
*/
static_assert(offsetof(struct ovl_layer, mnt) == 0);
static_assert(__same_type(typeof_member(struct ovl_layer, mnt), struct vfsmount *));
struct ovl_path { struct ovl_path {
const struct ovl_layer *layer; const struct ovl_layer *layer;
struct dentry *dentry; struct dentry *dentry;
......
...@@ -752,12 +752,12 @@ void ovl_free_fs(struct ovl_fs *ofs) ...@@ -752,12 +752,12 @@ void ovl_free_fs(struct ovl_fs *ofs)
if (ofs->upperdir_locked) if (ofs->upperdir_locked)
ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root); ovl_inuse_unlock(ovl_upper_mnt(ofs)->mnt_root);
/* Hack! Reuse ofs->layers as a vfsmount array before freeing it */ /* Reuse ofs->config.lowerdirs as a vfsmount array before freeing it */
mounts = (struct vfsmount **) ofs->layers; mounts = (struct vfsmount **) ofs->config.lowerdirs;
for (i = 0; i < ofs->numlayer; i++) { for (i = 0; i < ofs->numlayer; i++) {
iput(ofs->layers[i].trap); iput(ofs->layers[i].trap);
kfree(ofs->config.lowerdirs[i]);
mounts[i] = ofs->layers[i].mnt; mounts[i] = ofs->layers[i].mnt;
kfree(ofs->layers[i].name);
} }
kern_unmount_array(mounts, ofs->numlayer); kern_unmount_array(mounts, ofs->numlayer);
kfree(ofs->layers); kfree(ofs->layers);
...@@ -765,6 +765,7 @@ void ovl_free_fs(struct ovl_fs *ofs) ...@@ -765,6 +765,7 @@ void ovl_free_fs(struct ovl_fs *ofs)
free_anon_bdev(ofs->fs[i].pseudo_dev); free_anon_bdev(ofs->fs[i].pseudo_dev);
kfree(ofs->fs); kfree(ofs->fs);
kfree(ofs->config.lowerdirs);
kfree(ofs->config.upperdir); kfree(ofs->config.upperdir);
kfree(ofs->config.workdir); kfree(ofs->config.workdir);
if (ofs->creator_cred) if (ofs->creator_cred)
...@@ -949,16 +950,16 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry) ...@@ -949,16 +950,16 @@ int ovl_show_options(struct seq_file *m, struct dentry *dentry)
struct super_block *sb = dentry->d_sb; struct super_block *sb = dentry->d_sb;
struct ovl_fs *ofs = OVL_FS(sb); struct ovl_fs *ofs = OVL_FS(sb);
size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer; size_t nr, nr_merged_lower = ofs->numlayer - ofs->numdatalayer;
const struct ovl_layer *data_layers = &ofs->layers[nr_merged_lower]; char **lowerdatadirs = &ofs->config.lowerdirs[nr_merged_lower];
/* ofs->layers[0] is the upper layer */ /* lowerdirs[] starts from offset 1 */
seq_printf(m, ",lowerdir=%s", ofs->layers[1].name); seq_printf(m, ",lowerdir=%s", ofs->config.lowerdirs[1]);
/* dump regular lower layers */ /* dump regular lower layers */
for (nr = 2; nr < nr_merged_lower; nr++) for (nr = 2; nr < nr_merged_lower; nr++)
seq_printf(m, ":%s", ofs->layers[nr].name); seq_printf(m, ":%s", ofs->config.lowerdirs[nr]);
/* dump data lower layers */ /* dump data lower layers */
for (nr = 0; nr < ofs->numdatalayer; nr++) for (nr = 0; nr < ofs->numdatalayer; nr++)
seq_printf(m, "::%s", data_layers[nr].name); seq_printf(m, "::%s", lowerdatadirs[nr]);
if (ofs->config.upperdir) { if (ofs->config.upperdir) {
seq_show_option(m, "upperdir", ofs->config.upperdir); seq_show_option(m, "upperdir", ofs->config.upperdir);
seq_show_option(m, "workdir", ofs->config.workdir); seq_show_option(m, "workdir", ofs->config.workdir);
......
...@@ -104,8 +104,8 @@ static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak) ...@@ -104,8 +104,8 @@ static int ovl_revalidate_real(struct dentry *d, unsigned int flags, bool weak)
static int ovl_dentry_revalidate_common(struct dentry *dentry, static int ovl_dentry_revalidate_common(struct dentry *dentry,
unsigned int flags, bool weak) unsigned int flags, bool weak)
{ {
struct ovl_entry *oe = OVL_E(dentry); struct ovl_entry *oe;
struct ovl_path *lowerstack = ovl_lowerstack(oe); struct ovl_path *lowerstack;
struct inode *inode = d_inode_rcu(dentry); struct inode *inode = d_inode_rcu(dentry);
struct dentry *upper; struct dentry *upper;
unsigned int i; unsigned int i;
...@@ -115,6 +115,8 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry, ...@@ -115,6 +115,8 @@ static int ovl_dentry_revalidate_common(struct dentry *dentry,
if (!inode) if (!inode)
return -ECHILD; return -ECHILD;
oe = OVL_I_E(inode);
lowerstack = ovl_lowerstack(oe);
upper = ovl_i_dentry_upper(inode); upper = ovl_i_dentry_upper(inode);
if (upper) if (upper)
ret = ovl_revalidate_real(upper, flags, weak); ret = ovl_revalidate_real(upper, flags, weak);
...@@ -167,6 +169,7 @@ static void ovl_free_inode(struct inode *inode) ...@@ -167,6 +169,7 @@ static void ovl_free_inode(struct inode *inode)
struct ovl_inode *oi = OVL_I(inode); struct ovl_inode *oi = OVL_I(inode);
kfree(oi->redirect); kfree(oi->redirect);
kfree(oi->oe);
mutex_destroy(&oi->lock); mutex_destroy(&oi->lock);
kmem_cache_free(ovl_inode_cachep, oi); kmem_cache_free(ovl_inode_cachep, oi);
} }
...@@ -176,7 +179,7 @@ static void ovl_destroy_inode(struct inode *inode) ...@@ -176,7 +179,7 @@ static void ovl_destroy_inode(struct inode *inode)
struct ovl_inode *oi = OVL_I(inode); struct ovl_inode *oi = OVL_I(inode);
dput(oi->__upperdentry); dput(oi->__upperdentry);
ovl_free_entry(oi->oe); ovl_stack_put(ovl_lowerstack(oi->oe), ovl_numlower(oi->oe));
if (S_ISDIR(inode->i_mode)) if (S_ISDIR(inode->i_mode))
ovl_dir_cache_free(inode); ovl_dir_cache_free(inode);
else else
...@@ -569,11 +572,6 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs, ...@@ -569,11 +572,6 @@ static int ovl_get_upper(struct super_block *sb, struct ovl_fs *ofs,
upper_layer->idx = 0; upper_layer->idx = 0;
upper_layer->fsid = 0; upper_layer->fsid = 0;
err = -ENOMEM;
upper_layer->name = kstrdup(ofs->config.upperdir, GFP_KERNEL);
if (!upper_layer->name)
goto out;
/* /*
* Inherit SB_NOSEC flag from upperdir. * Inherit SB_NOSEC flag from upperdir.
* *
...@@ -1122,7 +1120,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs, ...@@ -1122,7 +1120,8 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
layers[ofs->numlayer].idx = ofs->numlayer; layers[ofs->numlayer].idx = ofs->numlayer;
layers[ofs->numlayer].fsid = fsid; layers[ofs->numlayer].fsid = fsid;
layers[ofs->numlayer].fs = &ofs->fs[fsid]; layers[ofs->numlayer].fs = &ofs->fs[fsid];
layers[ofs->numlayer].name = l->name; /* Store for printing lowerdir=... in ovl_show_options() */
ofs->config.lowerdirs[ofs->numlayer] = l->name;
l->name = NULL; l->name = NULL;
ofs->numlayer++; ofs->numlayer++;
ofs->fs[fsid].is_lower = true; ofs->fs[fsid].is_lower = true;
...@@ -1367,8 +1366,16 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc) ...@@ -1367,8 +1366,16 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
if (!layers) if (!layers)
goto out_err; goto out_err;
ofs->config.lowerdirs = kcalloc(ctx->nr + 1, sizeof(char *), GFP_KERNEL);
if (!ofs->config.lowerdirs) {
kfree(layers);
goto out_err;
}
ofs->layers = layers; ofs->layers = layers;
/* Layer 0 is reserved for upper even if there's no upper */ /*
* Layer 0 is reserved for upper even if there's no upper.
* For consistency, config.lowerdirs[0] is NULL.
*/
ofs->numlayer = 1; ofs->numlayer = 1;
sb->s_stack_depth = 0; sb->s_stack_depth = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment