linux-kernel - Re: [PATCH v3 -next] mm/hugetlb_cgroup: register lockdep key for cftype

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 19 Jun 2024 17:17:02 +0200
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Xiu Jianfeng <xiujianfeng@...wei.com>, tj@...nel.org,
	lizefan.x@...edance.com, hannes@...xchg.org, akpm@...ux-foundation.org
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org, sj@...nel.org
Subject: Re: [PATCH v3 -next] mm/hugetlb_cgroup: register lockdep key for
 cftype

On 19.06.2024 03:55, Xiu Jianfeng wrote:
> When CONFIG_DEBUG_LOCK_ALLOC is enabled, the following commands can
> trigger a bug,
>
> mount -t cgroup2 none /sys/fs/cgroup
> cd /sys/fs/cgroup
> echo "+hugetlb" > cgroup.subtree_control
>
> The log is as below:
>
> BUG: key ffff8880046d88d8 has not been registered!
> ------------[ cut here ]------------
> DEBUG_LOCKS_WARN_ON(1)
> WARNING: CPU: 3 PID: 226 at kernel/locking/lockdep.c:4945 lockdep_init_map_type+0x185/0x220
> Modules linked in:
> CPU: 3 PID: 226 Comm: bash Not tainted 6.10.0-rc4-next-20240617-g76db4c64526c #544
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> RIP: 0010:lockdep_init_map_type+0x185/0x220
> Code: 00 85 c0 0f 84 6c ff ff ff 8b 3d 6a d1 85 01 85 ff 0f 85 5e ff ff ff 48 c7 c6 21 99 4a 82 48 c7 c7 60 29 49 82 e8 3b 2e f5
> RSP: 0018:ffffc9000083fc30 EFLAGS: 00000282
> RAX: 0000000000000000 RBX: ffffffff828dd820 RCX: 0000000000000027
> RDX: ffff88803cd9cac8 RSI: 0000000000000001 RDI: ffff88803cd9cac0
> RBP: ffff88800674fbb0 R08: ffffffff828ce248 R09: 00000000ffffefff
> R10: ffffffff8285e260 R11: ffffffff828b8eb8 R12: ffff8880046d88d8
> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880067281c0
> FS:  00007f68601ea740(0000) GS:ffff88803cd80000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005614f3ebc740 CR3: 000000000773a000 CR4: 00000000000006f0
> Call Trace:
>   <TASK>
>   ? __warn+0x77/0xd0
>   ? lockdep_init_map_type+0x185/0x220
>   ? report_bug+0x189/0x1a0
>   ? handle_bug+0x3c/0x70
>   ? exc_invalid_op+0x18/0x70
>   ? asm_exc_invalid_op+0x1a/0x20
>   ? lockdep_init_map_type+0x185/0x220
>   __kernfs_create_file+0x79/0x100
>   cgroup_addrm_files+0x163/0x380
>   ? find_held_lock+0x2b/0x80
>   ? find_held_lock+0x2b/0x80
>   ? find_held_lock+0x2b/0x80
>   css_populate_dir+0x73/0x180
>   cgroup_apply_control_enable+0x12f/0x3a0
>   cgroup_subtree_control_write+0x30b/0x440
>   kernfs_fop_write_iter+0x13a/0x1f0
>   vfs_write+0x341/0x450
>   ksys_write+0x64/0xe0
>   do_syscall_64+0x4b/0x110
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7f68602d9833
> Code: 8b 15 61 26 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 08
> RSP: 002b:00007fff9bbdf8e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f68602d9833
> RDX: 0000000000000009 RSI: 00005614f3ebc740 RDI: 0000000000000001
> RBP: 00005614f3ebc740 R08: 000000000000000a R09: 0000000000000008
> R10: 00005614f3db6ba0 R11: 0000000000000246 R12: 0000000000000009
> R13: 00007f68603bd6a0 R14: 0000000000000009 R15: 00007f68603b8880
>
> For lockdep, there is a sanity check in lockdep_init_map_type(), the
> lock-class key must either have been allocated statically or must
> have been registered as a dynamic key. However the commit e18df2889ff9
> ("mm/hugetlb_cgroup: prepare cftypes based on template") has changed
> the cftypes from static allocated objects to dynamic allocated objects,
> so the cft->lockdep_key must be registered proactively.
>
> Fixes: e18df2889ff9 ("mm/hugetlb_cgroup: prepare cftypes based on template")
> Reported-by: kernel test robot <oliver.sang@...el.com>
> Closes: https://lore.kernel.org/oe-lkp/202406181046.8d8b2492-oliver.sang@intel.com
> Signed-off-by: Xiu Jianfeng <xiujianfeng@...wei.com>

Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>

Fixes the issue pointed here:

https://lore.kernel.org/all/602186b3-5ce3-41b3-90a3-134792cc2a48@samsung.com/

> ---
> v3: remove #ifdef CONFIG_DEBUG_LOCK_ALLOC in struct cftype
> v2: add bug log to commit message
> ---
>   include/linux/cgroup-defs.h | 2 --
>   mm/hugetlb_cgroup.c         | 2 ++
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
> index b36690ca0d3f..293af7f8a694 100644
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -681,9 +681,7 @@ struct cftype {
>   	__poll_t (*poll)(struct kernfs_open_file *of,
>   			 struct poll_table_struct *pt);
>   
> -#ifdef CONFIG_DEBUG_LOCK_ALLOC
>   	struct lock_class_key	lockdep_key;
> -#endif
>   };
>   
>   /*
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index 2b899c4ae968..4ff238ba1250 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -836,6 +836,8 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft,
>   			cft->file_offset = MEMFILE_OFFSET0(offset) +
>   					   MEMFILE_FIELD_SIZE(offset) * idx;
>   		}
> +
> +		lockdep_register_key(&cft->lockdep_key);
>   	}
>   }
>   

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland