On Mon, 18 Oct 2021 19:51:49 +0300 Nikita Yushchenko <nikita.yushche...@virtuozzo.com> wrote:
> The assumption that bm_fill_super() is not called for the second time > for CT is wrong: umount operation clears sb->s_root, which causes > vfs_get_super() to call fill_super again on the next mount. > > Make bm_fill_super() handle multiple-calls corrently: > - initialize bm_data and set ve->binfmt_misc only if it is not done > before, > - delay desctruction of it up to CT destruction. > > https://jira.sw.ru/browse/PSBM-133968 > Fixes: 8250ff41d190 ("ve/fs/binfmt: clean bm_data reference from ve on err > path") > Signed-off-by: Nikita Yushchenko <nikita.yushche...@virtuozzo.com> > --- > fs/binfmt_misc.c | 37 +++++++++++++------------------------ > 1 file changed, 13 insertions(+), 24 deletions(-) > > diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c > index 0946e7e6caa5..5f4e90c1ade2 100644 > --- a/fs/binfmt_misc.c > +++ b/fs/binfmt_misc.c > @@ -853,38 +853,25 @@ static int bm_fill_super(struct super_block *sb, struct > fs_context *fc) > }; > > struct ve_struct *ve = get_exec_env(); > - struct binfmt_misc *bm_data; > + struct binfmt_misc *bm_data = ve->binfmt_misc; > > - /* > - * bm_get_tree() > - * get_tree_keyed(fc, bm_fill_super, get_ve(ve)) > - * fc->s_fs_info = current VE > - * vfs_get_super(fc, vfs_get_keyed_super, bm_fill_super) > - * sb = sget_fc(fc, test, set_anon_super_fc) > - * if (!sb->s_root) { > - * err = bm_fill_super(sb, fc); > - * > - * => we should never get here with initialized ve->binfmt_misc. > - */ > - if (WARN_ON_ONCE(ve->binfmt_misc)) > - return -EEXIST; > + if (!bm_data) { > + bm_data = kzalloc(sizeof(struct binfmt_misc), GFP_KERNEL); > + if (!bm_data) > + return -ENOMEM; > > - bm_data = kzalloc(sizeof(struct binfmt_misc), GFP_KERNEL); > - if (!bm_data) > - return -ENOMEM; > + INIT_LIST_HEAD(&bm_data->entries); > + rwlock_init(&bm_data->entries_lock); > > - INIT_LIST_HEAD(&bm_data->entries); > - rwlock_init(&bm_data->entries_lock); > + ve->binfmt_misc = bm_data; Isn't it better to move ve->binfmt_misc assignment to the end of function where we know that all operations was successful? > + /* this will be cleared by ve_binfmt_fini() */ > + } > > err = simple_fill_super(sb, BINFMTFS_MAGIC, bm_files); > - if (err) { > - kfree(bm_data); If we have ve->binfmt_misc assignment in the upper part of code, then we need to do ve->binfmt_misc = NULL here. > + if (err) > return err; > - } > > sb->s_op = &s_ops; > - > - ve->binfmt_misc = bm_data; see above > bm_data->enabled = 1; > > return 0; > @@ -971,6 +958,8 @@ static void ve_binfmt_fini(void *data) > while (!list_empty(&bm_data->entries)) > kill_node(bm_data, list_first_entry( > &bm_data->entries, Node, list)); > + > + kfree(bm_data); We have kfree in ve_destroy (kernel/ve/ve.c) already. > } > > static struct ve_hook ve_binfmt_hook = { > -- > 2.30.2 > _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
