[PATCH] EDAC/amd64: Fix double free of private data on registration failure

[PATCH] EDAC/amd64: Fix double free of private data on registration failure

[Weilin Tong]

If edac_mc_add_mc_with_groups() fails, init_one_instance() calls
edac_mc_free(), which frees mci->pvt_info. probe_one_instance() then
accesses and frees the same private data, causing a use-after-free
and a double free.

Clear mci->pvt_info before calling edac_mc_free() to leave the cleanup
to probe_one_instance(). The normal remove path already clears it.

Fixes: 0bbb265f7089 ("EDAC/mc: Get rid of silly one-shot struct allocation in edac_mc_alloc()")
Cc: stable@vger.kernel.org
Reported-by: Qinyun Tan <qinyuntan@linux.alibaba.com>
Signed-off-by: Weilin Tong <tongweilin@linux.alibaba.com>
---
 drivers/edac/amd64_edac.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 0c0d11c72925..edced7cbe83f 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3975,6 +3975,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	ret = -ENODEV;
 	if (edac_mc_add_mc_with_groups(mci, amd64_edac_attr_groups)) {
 		edac_dbg(1, "failed edac_mc_add_mc()\n");
+		mci->pvt_info = NULL;
 		edac_mc_free(mci);
 		return ret;
 	}

base-commit: 2c455ddca7827ad8fa3a64fdbf5a60efb928b46a
-- 
2.43.5

Re: [PATCH] EDAC/amd64: Fix double free of private data on registration failure

[Yazen Ghannam]

On Wed, Oct 21, 2026 at 02:29:20PM +0800, Weilin Tong wrote:
> If edac_mc_add_mc_with_groups() fails, init_one_instance() calls
> edac_mc_free(), which frees mci->pvt_info. probe_one_instance() then
> accesses and frees the same private data, causing a use-after-free
> and a double free.
> 
> Clear mci->pvt_info before calling edac_mc_free() to leave the cleanup
> to probe_one_instance(). The normal remove path already clears it.
> 
> Fixes: 0bbb265f7089 ("EDAC/mc: Get rid of silly one-shot struct allocation in edac_mc_alloc()")
> Cc: stable@vger.kernel.org
> Reported-by: Qinyun Tan <qinyuntan@linux.alibaba.com>
> Signed-off-by: Weilin Tong <tongweilin@linux.alibaba.com>
> ---
>  drivers/edac/amd64_edac.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 0c0d11c72925..edced7cbe83f 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3975,6 +3975,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
>  	ret = -ENODEV;
>  	if (edac_mc_add_mc_with_groups(mci, amd64_edac_attr_groups)) {
>  		edac_dbg(1, "failed edac_mc_add_mc()\n");
> +		mci->pvt_info = NULL;
>  		edac_mc_free(mci);
>  		return ret;
>  	}
> 
> base-commit: 2c455ddca7827ad8fa3a64fdbf5a60efb928b46a
> -- 

Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>

Thanks,
Yazen

Re: [PATCH] EDAC/amd64: Fix double free of private data on registration failure

[Borislav Petkov]

On Wed, Oct 21, 2026 at 02:29:20PM +0800, Weilin Tong wrote:
> If edac_mc_add_mc_with_groups() fails, init_one_instance() calls
> edac_mc_free(), which frees mci->pvt_info. probe_one_instance() then
> accesses and frees the same private data, causing a use-after-free
> and a double free.
> 
> Clear mci->pvt_info before calling edac_mc_free() to leave the cleanup
> to probe_one_instance(). The normal remove path already clears it.
> 
> Fixes: 0bbb265f7089 ("EDAC/mc: Get rid of silly one-shot struct allocation in edac_mc_alloc()")
> Cc: stable@vger.kernel.org

Stable because?

Are you triggering this or some tool/AI caught it?

> Reported-by: Qinyun Tan <qinyuntan@linux.alibaba.com>
> Signed-off-by: Weilin Tong <tongweilin@linux.alibaba.com>
> ---
>  drivers/edac/amd64_edac.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
> index 0c0d11c72925..edced7cbe83f 100644
> --- a/drivers/edac/amd64_edac.c
> +++ b/drivers/edac/amd64_edac.c
> @@ -3975,6 +3975,7 @@ static int init_one_instance(struct amd64_pvt *pvt)
>  	ret = -ENODEV;
>  	if (edac_mc_add_mc_with_groups(mci, amd64_edac_attr_groups)) {

What a mess this code is with passing pvt up and down the chain and making it
so easy for people to mess up the responsibilities of who is supposed to free
allocated memory... :-\

>  		edac_dbg(1, "failed edac_mc_add_mc()\n");

In any case, this line needs a comment explaining why this ptr is getting
cleared because it is not trivial just from staring at it.

Something about probe_one_instance() as the one who allocates the memory, how
that one should be the only one which frees that same memory.

> +		mci->pvt_info = NULL;
>  		edac_mc_free(mci);
>  		return ret;
>  	}

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH v2] EDAC/amd64: Fix double free of private data on registration failure

[Weilin Tong]

If edac_mc_add_mc_with_groups() fails, init_one_instance() calls
edac_mc_free(), which frees mci->pvt_info. probe_one_instance() then
accesses and frees the same private data, causing a use-after-free
and a double free.

Clear mci->pvt_info before calling edac_mc_free() to leave the cleanup
to probe_one_instance(). The normal remove path already clears it.

Fixes: 0bbb265f7089 ("EDAC/mc: Get rid of silly one-shot struct allocation in edac_mc_alloc()")
Reported-by: Qinyun Tan <qinyuntan@linux.alibaba.com>
Signed-off-by: Weilin Tong <tongweilin@linux.alibaba.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
v2:
- Explain why probe_one_instance() retains ownership of pvt on failure.
- Drop the stable Cc.
- Add Yazen's Reviewed-by.
- Rebase onto ras/edac-for-next (28f141ff14c48).

v1: https://lore.kernel.org/linux-edac/20261021062925.1838750-1-tongweilin@linux.alibaba.com/

 drivers/edac/amd64_edac.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index a8e76a79833b..d4abc5200ea1 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -3973,6 +3973,11 @@ static int init_one_instance(struct amd64_pvt *pvt)
 	ret = -ENODEV;
 	if (edac_mc_add_mc_with_groups(mci, amd64_edac_attr_groups)) {
 		edac_dbg(1, "failed edac_mc_add_mc()\n");
+		/*
+		 * probe_one_instance() allocates pvt and frees it on failure.
+		 * Keep edac_mc_free() from freeing it here.
+		 */
+		mci->pvt_info = NULL;
 		edac_mc_free(mci);
 		return ret;
 	}

base-commit: 28f141ff14c48abd38518a87d1026851dd12717e
-- 
2.43.7