linux-kernel - Re: [PATCH next] mm/mempolicy: Fix error code in sysfs_wi_node

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]

Message-ID: <20250423152805.3356081-1-joshua.hahnjy@gmail.com>
Date: Wed, 23 Apr 2025 08:27:33 -0700
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Rakie Kim <rakie.kim@...com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Honggyu Kim <honggyu.kim@...com>,
	Dan Williams <dan.j.williams@...el.com>,
	Gregory Price <gourry@...rry.net>,
	linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH next] mm/mempolicy: Fix error code in sysfs_wi_node_add()

On Wed, 23 Apr 2025 11:24:58 +0300 Dan Carpenter <dan.carpenter@...aro.org> wrote:

Hi Dan,

This makes sense to me! I think that the only way the node can already exist
is if an offlining node didn't properly clean up its sysfs entry, which is
a bug, of course. With that said, I don't think the previous state would have
caused any functional problems, since the same node offlining and onlining
should share the same sysfs entry anyways (unless I'm overlooking something
important...)

This fix will help when the cleanup does fail though, and I think that will
help us assess whether a failed cleanup does indeed cause other problems.

Thank you for spotting this & fixing it! I hope you have a great day : -)

Reviewed-by: Joshua Hahn <joshua.hahnjy@...il.com>

> Return -EEXIST if the node already exists.  Don't return success.
> 
> Fixes: 1bf270ac1b0a ("mm/mempolicy: support memory hotplug in weighted interleave")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>
> ---
> Potentially returning success was intentional?  This is from static
> analysis and I can't be totally sure.
> 
>  mm/mempolicy.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index f43951668c41..0538a994440a 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -3539,7 +3539,7 @@ static const struct kobj_type wi_ktype = {
>  
>  static int sysfs_wi_node_add(int nid)
>  {
> -	int ret = 0;
> +	int ret;
>  	char *name;
>  	struct iw_node_attr *new_attr;
>  
> @@ -3569,6 +3569,7 @@ static int sysfs_wi_node_add(int nid)
>  	if (wi_group->nattrs[nid]) {
>  		mutex_unlock(&wi_group->kobj_lock);
>  		pr_info("node%d already exists\n", nid);
> +		ret = -EEXIST;
>  		goto out;
>  	}
>  
> -- 
> 2.47.2