[Devel] [PATCH RHEL8 COMMIT] ve: Fix double-free if cgroup_mark_ve_roots() fails

[Konstantin Khorenko]

The commit is pushed to "branch-rh8-4.18.0-240.1.1.vz8.5.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh8-4.18.0-240.1.1.vz8.5.13
------>
commit a288f3ade0ade86a656546f6a647cd556b02f36f
Author: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>
Date:   Fri Apr 9 20:29:57 2021 +0300

    ve: Fix double-free if cgroup_mark_ve_roots() fails
    
    Below is a dmesg output when cgroup_mark_ve_roots exits with error.
    
    [  +0.973416] ============================================================
    [  +0.000140] BUG: KASAN: double-free or invalid-free in kfree+0xd6/0x2d0
    
    [  +0.000088] CPU: 0 PID: 285 Comm: kworker/0:3 ve: / Not tainted
    [  +0.000002] Hardware name: Virtuozzo KVM, BIOS
    [  +0.000017] Workqueue: cgroup_destroy css_killed_work_fn
    [  +0.000007] Call Trace:
    [  +0.000019]  dump_stack+0x9a/0xf0
    [  +0.000006]  print_address_description.cold.3+0x9/0x23b
    [  +0.000004]  ? kfree+0xd6/0x2d0
    [  +0.000005]  kasan_report_invalid_free+0x65/0xa0
    [  +0.000004]  ? kfree+0xd6/0x2d0
    [  +0.000003]  __kasan_slab_free+0x157/0x170
    [  +0.000006]  slab_free_freelist_hook+0x5e/0x140
    [  +0.000012]  ? ve_offline+0x34/0x70
    [  +0.000005]  kfree+0xd6/0x2d0
    [  +0.000007]  ve_offline+0x34/0x70
    [  +0.000004]  css_killed_work_fn+0xa5/0x490
    [  +0.000015]  process_one_work+0x8f0/0x17a0
    [  +0.000013]  ? pwq_dec_nr_in_flight+0x320/0x320
    [  +0.000003]  ? lock_acquire+0x14f/0x3b0
    [  +0.000015]  worker_thread+0x87/0xb50
    [  +0.000010]  ? __kthread_parkme+0xb6/0x180
    [  +0.000006]  ? process_one_work+0x17a0/0x17a0
    [  +0.000004]  kthread+0x30e/0x3d0
    [  +0.000003]  ? kthread_create_fn+0x70/0x70
    [  +0.000012]  ret_from_fork+0x3a/0x50
    
    [  +0.000036] Allocated by task 2817:
    [  +0.000038]  kasan_kmalloc+0xbf/0xe0
    [  +0.000003]  __kmalloc+0x157/0x320
    [  +0.000022]  ext4_htree_store_dirent+0x88/0x570 [ext4]
    [  +0.000018]  htree_dirblock_to_tree+0x235/0x540 [ext4]
    [  +0.000016]  ext4_htree_fill_tree+0x1e0/0x880 [ext4]
    [  +0.000014]  ext4_readdir+0xf5e/0x2910 [ext4]
    [  +0.000010]  iterate_dir+0x3b0/0x610
    [  +0.000003]  ksys_getdents64+0x11f/0x1f0
    [  +0.000003]  __x64_sys_getdents64+0x6f/0xb0
    [  +0.000004]  do_syscall_64+0xa5/0x4d0
    [  +0.000006]  entry_SYSCALL_64_after_hwframe+0x6a/0xdf
    [  +0.000003]  0xffffffffffffffff
    
    [  +0.000022] Freed by task 2817:
    [  +0.000034]  __kasan_slab_free+0x125/0x170
    [  +0.000003]  slab_free_freelist_hook+0x5e/0x140
    [  +0.000003]  kfree+0xd6/0x2d0
    [  +0.000015]  free_rb_tree_fname+0x67/0xb0 [ext4]
    [  +0.000014]  ext4_release_dir+0x3c/0x60 [ext4]
    [  +0.000003]  __fput+0x272/0x7b0
    [  +0.000003]  task_work_run+0x115/0x180
    [  +0.000003]  exit_to_usermode_loop+0x152/0x170
    [  +0.000003]  do_syscall_64+0x41e/0x4d0
    [  +0.000003]  entry_SYSCALL_64_after_hwframe+0x6a/0xdf
    [  +0.000003]  0xffffffffffffffff
    
    [  +0.000022] The buggy address belongs to the object at ffff888107da8b80
                   which belongs to the cache kmalloc-64 of size 64
    [  +0.000120] The buggy address is located 0 bytes inside of
                   64-byte region [ffff888107da8b80, ffff888107da8bc0)
    [  +0.000107] The buggy address belongs to the page:
    [  +0.000054] page:ffffea00041f6a00 refcount:1 mapcount:0
                  mapping:ffff888107c0f000 index:0xffff888107da8b80
    [  +0.000004] flags: 0x17ffffc0000100(slab)
    [  +0.000004] raw: 0017ffffc0000100 ffffea000436a0c8 ffffea000428c748
    [  +0.000003] raw: ffff888107da8b80 000000000020001f 00000001ffffffff
    [  +0.000002] page dumped because: kasan: bad access detected
    
    [  +0.000021] Memory state around the buggy address:
    [  +0.000117]  ffff888107da8a80: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc
    [  +0.000071]  ffff888107da8b00: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
    [  +0.000103] >ffff888107da8b80: fb fb fb fb fb fb fb fb fc fc fc fc fc fc
    [  +0.000102]                    ^
    [  +0.000035]  ffff888107da8c00: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc
    [  +0.000069]  ffff888107da8c80: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
    [  +0.000069]
    ==================================================================
    
    When this happens, ve_start_container() proceeds to error path and does
    kfree(ve->ve_owner). The same kfree is later done in ve_offline() function.
    
    v2 changes by khorenko@:
    in fact the "kfree(ve->ve_owner)" in ve_start_container() error path is
    redundant because ve_offline() is always called in case ve_online()
    succeeds (i.e. ve::ve_name is allocated).
    
    So drop the redundant kfree(ve->ve_name).
    
    Signed-off-by: Valeriy Vdovin <valeriy.vdovin at virtuozzo.com>
    Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 kernel/ve/ve.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 8dbf56e78bb9..41e81aec3822 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -656,7 +656,6 @@ static int ve_start_container(struct ve_struct *ve)
 	ve_list_del(ve);
 err_list:
 	ve_drop_context(ve);
-	kfree(ve->ve_name);
 	return err;
 }