Re: [PATCH] x86/resctrl: Fix invalid attempt at removing default resource group

[Reinette Chatre]

Hi Thomas and Borislav,

Could you please consider this patch for inclusion as a fix for v5.7?

Thank you

Reinette


On 3/17/2020 9:26 AM, Reinette Chatre wrote:
> The default resource group ("rdtgroup_default") is associated with the
> root of the resctrl filesystem and should never be removed. New resource
> groups can be created as subdirectories of the resctrl filesystem and
> they can be removed from user space. There exists a safeguard in the
> directory removal code (rdtgroup_rmdir()) that ensures that only
> subdirectories can be removed by testing that the directory to be
> removed has to be a child of the root directory.
> 
> A possible deadlock was recently fixed with commit 334b0f4e9b1b
> ("x86/resctrl: Fix a deadlock due to inaccurate reference"). This fix
> involved associating the private data of the "mon_groups" and "mon_data"
> directories to the resource group to which they belong instead of NULL
> as before. A consequence of this change was that the original safeguard
> code preventing removal of "mon_groups" and "mon_data" found in the root
> directory failed resulting in attempts to remove the default resource
> group that ends in a BUG:
> 
> kernel BUG at mm/slub.c:3969!
> invalid opcode: 0000 [#1] SMP PTI
> 
> Call Trace:
> rdtgroup_rmdir+0x16b/0x2c0
> kernfs_iop_rmdir+0x5c/0x90
> vfs_rmdir+0x7a/0x160
> do_rmdir+0x17d/0x1e0
> do_syscall_64+0x55/0x1d0
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> Fix this by improving the directory removal safeguard to ensure that
> subdirectories of the resctrl root directory can only be removed if
> they are a child of the resctrl filesystem's root _and_ not associated
> with the default resource group.
> 
> Fixes: 334b0f4e9b1b ("x86/resctrl: Fix a deadlock due to inaccurate reference")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@xxxxxxxxx>
> Tested-by: Sai Praneeth Prakhya <sai.praneeth.prakhya@xxxxxxxxx>
> Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 064e9ef44cd6..9d4e73a9b5a9 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -3072,7 +3072,8 @@ static int rdtgroup_rmdir(struct kernfs_node *kn)
>  	 * If the rdtgroup is a mon group and parent directory
>  	 * is a valid "mon_groups" directory, remove the mon group.
>  	 */
> -	if (rdtgrp->type == RDTCTRL_GROUP && parent_kn == rdtgroup_default.kn) {
> +	if (rdtgrp->type == RDTCTRL_GROUP && parent_kn == rdtgroup_default.kn &&
> +	    rdtgrp != &rdtgroup_default) {
>  		if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP ||
>  		    rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED) {
>  			ret = rdtgroup_ctrl_remove(kn, rdtgrp);
>