On 9/6/25 01:03, Konstantin Khorenko wrote:
If we fail to alloc ve_struct, we'll crash here on setting VE state.
Moving ve_set_state() earlier makes err_ve: label redundant,
so get rid of it according to coding style rules.
Also let's set VE_STATE_DEAD state right before freeing ve_struct
instead of VE_STATE_STOPPED, it will be more logical to see _DEAD state
in the freed/not used memory and it corresponds to other cases when
VE_STATE_DEAD is set:
ve_start_container()
err handling:
ve_set_state(ve, VE_STATE_STOPPED);
ve_drop_context(ve); // not freeing memory
ve_exit_ns
ve_set_state(ve, VE_STATE_STOPPED);
put_ve(ve); /* from ve_start_container() */
// no memory free here, only later in ve_destroy()
ve_destroy
ve_set_state(ve, VE_STATE_DEAD);
kmem_cache_free(ve_cachep, ve); // freeing memory
ve_create
err handling:
ve_set_state(ve, VE_STATE_DEAD); // logical to use _DEAD here
kmem_cache_free(ve_cachep, ve); // freeing memory as well
Agreed, thanks for explaining this one in detail!
Fixes: 666e40b308457 ("ve/cgroups: Drop lock when stopping workqueue to
avoid dead lock")
Signed-off-by: Konstantin Khorenko <khore...@virtuozzo.com>
Feature: ve: ve generic structures
---
v2:
* drop err_ve: label
kernel/ve/ve.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 146f7922d4856..663c1c2255621 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -951,10 +951,9 @@ static struct cgroup_subsys_state *ve_create(struct
cgroup_subsys_state *parent_
if (css_to_ve(parent_css) != &ve0)
return ERR_PTR(-ENOTDIR);
- err = -ENOMEM;
Due to removal of the above err variable initialization, we can end up
using uninitialized err in "err_lat" error path.
ve = kmem_cache_zalloc(ve_cachep, GFP_KERNEL);
if (!ve)
- goto err_ve;
+ return ERR_PTR(-ENOMEM);
ve->sched_lat_ve.cur = alloc_percpu(struct kstat_lat_pcpu_snap_struct);
if (!ve->sched_lat_ve.cur)
@@ -1019,7 +1018,6 @@ static struct cgroup_subsys_state *ve_create(struct
cgroup_subsys_state *parent_
ve->aio_nr = 0;
ve->aio_max_nr = AIO_MAX_NR_DEFAULT;
#endif
-
return &ve->css;
err_vdso:
@@ -1028,9 +1026,9 @@ static struct cgroup_subsys_state *ve_create(struct
cgroup_subsys_state *parent_
err_log:
free_percpu(ve->sched_lat_ve.cur);
err_lat:
+ ve_set_state(ve, VE_STATE_DEAD);
kmem_cache_free(ve_cachep, ve);
-err_ve:
- ve_set_state(ve, VE_STATE_STOPPED);
+
return ERR_PTR(err);
}
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
