linux-kernel - Re: [PATCH] driver:fix a bug of function remove

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] [day] [month] [year] [list]

Message-ID: <2025052143-rubble-scalded-b04e@gregkh>
Date: Wed, 21 May 2025 14:33:04 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Changhang Luo <luochanghang_nudt@....com>
Cc: rafael@...nel.org, dakr@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] driver:fix a bug of function remove_nodes in
 "drivers/base/devres.c"

On Sat, May 03, 2025 at 04:04:54PM +0800, Changhang Luo wrote:
> The resource management mechanism in Linux divides resources 
> into different groups for easier management. Each closed group
> has a start and an close resource node to represent the scope
> of the resource group. The function "remove_nodes" is try to move
> the nodes in the same group into the todo list. Fisrt the function
> moves the normal nodes into the todo list, then scan the left node
> and color the group value of two if the start and close resource
> nodes contained in [current node, end).
> 
> While, in the second pass section here is:
>   if (list_empty(&grp->node[1].entry))
>      grp->color++;
> which means that the color is set to 2 when the grp_node[1] is not
> in the list.This situation is inconsistent with the original design
> intention that color value is set to 2 if the start and close nodes
>  both contained in [current node, end).
> 
> So it should be
>   if (!list_empty(&grp->node[1].entry))
>     grep->color++;
> that the grep->node[1] is verified to be contained 
> [current node, end) and then color is increased to 2.
> 
> Finally,
>   if(grp->color ==2){
>     list_move_tail(&grp->node[0].entry,tode);
>     list_del_init(&grp->node[1].entry);
>   }
> the closed group can be removed currectly.
> 
> then
>   list_del_init(grep->node[1].entry);
> while
> 
> Signed-off-by: Changhang Luo <luochanghang_nudt@....com>
> ---
>  drivers/base/devres.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
> index d8a733ea5..68f64256a 100644
> --- a/drivers/base/devres.c
> +++ b/drivers/base/devres.c
> @@ -477,7 +477,7 @@ static int remove_nodes(struct device *dev,
>  		BUG_ON(!grp || list_empty(&grp->node[0].entry));
>  
>  		grp->color++;
> -		if (list_empty(&grp->node[1].entry))
> +		if (!list_empty(&grp->node[1].entry))
>  			grp->color++;

Are you sure about this?  How was this tested?  Why hasn't it shown up
before?  And what commit id does this "fix"?

thanks,

greg k-h