[PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Wentao Liang]

In i10nm_get_ddr_munits(), get_ddr_munit() returns a PCI device with
its reference count incremented.  If ioremap() fails to map the MMIO
space of the memory controller, the function returns -ENODEV without
putting the reference held by mdev, leaking it.

Put the device reference before returning on the ioremap failure path.

Fixes: c545f5e41225 ("EDAC/i10nm: Skip the absent memory controllers")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/edac/i10nm_base.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c
index 63df35444214..b0cabe0daccd 100644
--- a/drivers/edac/i10nm_base.c
+++ b/drivers/edac/i10nm_base.c
@@ -847,6 +847,7 @@ static int i10nm_get_ddr_munits(void)
 
 			mbase = ioremap(base + off, size);
 			if (!mbase) {
+				pci_dev_put(mdev);
 				i10nm_printk(KERN_ERR, "Failed to ioremap 0x%llx\n",
 					     base + off);
 				return -ENODEV;
-- 
2.34.1

RE: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Leo Zhuo]

> From: Wentao Liang <vulab@iscas.ac.cn>
> Sent: Wednesday, September 16, 2026 5:13 PM
> To: bp@alien8.de
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org;
> qiuxu.zhuo@intel.com; tony.luck@intel.com; Wentao Liang <vulab@iscas.ac.cn>;
> stable@vger.kernel.org
> Subject: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()
> 
> External email: Use caution opening links or attachments
> 
> 
> In i10nm_get_ddr_munits(), get_ddr_munit() returns a PCI device with its reference
> count incremented.  If ioremap() fails to map the MMIO space of the memory
> controller, the function returns -ENODEV without putting the reference held by
> mdev, leaking it.
> 
> Put the device reference before returning on the ioremap failure path.
> 
> Fixes: c545f5e41225 ("EDAC/i10nm: Skip the absent memory controllers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

LGTM,

  Reviewed-by: Qiuxu Zhuo <qzhuo@nvidia.com>

[...]

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Luck, Tony]

On Wed, Sep 16, 2026 at 09:12:51AM +0000, Wentao Liang wrote:
> In i10nm_get_ddr_munits(), get_ddr_munit() returns a PCI device with
> its reference count incremented.  If ioremap() fails to map the MMIO
> space of the memory controller, the function returns -ENODEV without
> putting the reference held by mdev, leaking it.
> 
> Put the device reference before returning on the ioremap failure path.

Applied. Thanks.

-Tony

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Borislav Petkov]

Hi Tony,

On Thu, Sep 17, 2026 at 04:35:44PM -0700, Luck, Tony wrote:
> On Wed, Sep 16, 2026 at 09:12:51AM +0000, Wentao Liang wrote:
> > In i10nm_get_ddr_munits(), get_ddr_munit() returns a PCI device with
> > its reference count incremented.  If ioremap() fails to map the MMIO
> > space of the memory controller, the function returns -ENODEV without
> > putting the reference held by mdev, leaking it.
> > 
> > Put the device reference before returning on the ioremap failure path.
> 
> Applied. Thanks.

this one has CC:stable. That means it needs to go to Linus now. If so, it
needs to be into edac-urgent.

Yes? No?

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Luck, Tony]

On Fri, Sep 18, 2026 at 10:34:37PM -0700, Borislav Petkov wrote:
> Hi Tony,
> 
> On Thu, Sep 17, 2026 at 04:35:44PM -0700, Luck, Tony wrote:
> > On Wed, Sep 16, 2026 at 09:12:51AM +0000, Wentao Liang wrote:
> > > In i10nm_get_ddr_munits(), get_ddr_munit() returns a PCI device with
> > > its reference count incremented.  If ioremap() fails to map the MMIO
> > > space of the memory controller, the function returns -ENODEV without
> > > putting the reference held by mdev, leaking it.
> > > 
> > > Put the device reference before returning on the ioremap failure path.
> > 
> > Applied. Thanks.
> 
> this one has CC:stable. That means it needs to go to Linus now. If so, it
> needs to be into edac-urgent.

I though urgent was for:
1) Fixing something that broke in the current merge window
2) Fixing something that's really important (security, corruption)

This is just an old issue that's been around for ages.
> 
> Yes? No?

No.

-Tony

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Borislav Petkov]

Dropping stable@ from CC as it shouldn't be on CC.

On Mon, Sep 21, 2026 at 09:04:30AM -0700, Luck, Tony wrote:
> I though urgent was for:
> 1) Fixing something that broke in the current merge window
> 2) Fixing something that's really important (security, corruption)
> 
> This is just an old issue that's been around for ages.

If this patch has CC:stable and a Fixes: tag, then it'll get backported to
stable. So does it matter when it gets sent to Linus?

IOW, those two things do turn it into an urgent patch.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Luck, Tony]

On Mon, Sep 21, 2026 at 09:50:57PM -0700, Borislav Petkov wrote:
> Dropping stable@ from CC as it shouldn't be on CC.
> 
> On Mon, Sep 21, 2026 at 09:04:30AM -0700, Luck, Tony wrote:
> > I though urgent was for:
> > 1) Fixing something that broke in the current merge window
> > 2) Fixing something that's really important (security, corruption)
> > 
> > This is just an old issue that's been around for ages.
> 
> If this patch has CC:stable and a Fixes: tag, then it'll get backported to
> stable. So does it matter when it gets sent to Linus?
> 
> IOW, those two things do turn it into an urgent patch.

The "Fixes:" tag is a matter of fact. The "Cc: stable" is just an
opinion.

An extreme example would be a patch that fixes a spelling error in a
comment posted on a Friday before Linus is expected to tag a "final"
release in a cycle. A "Cc: stable" on that patch shouldn't trigger
sending to Linus via an urgent pull.

But if you want to add this patch to edac-urgent, then go ahead.

-Tony

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Borislav Petkov]

On Tue, Sep 22, 2026 at 07:59:56AM -0700, Luck, Tony wrote:
> The "Fixes:" tag is a matter of fact. The "Cc: stable" is just an
> opinion.

Are you being serious here?

"CC: stable" is a matter of opinion?!?!

Tony, where are you getting this from? Please point me to the docs where we
state that, please, because I have never read it before.

Maybe I have missed that memo, who knows?

All I know is that stable folks appreciate and more or less rely on CC:stable
from the patch committers so that they can know to pick up the patch quick. So
CC:stable is actually a statement: this patch should go to stable.

Fixes: is just a tag that shows which commit it fixes.
 
> An extreme example would be a patch that fixes a spelling error in a
> comment posted on a Friday before Linus is expected to tag a "final"
> release in a cycle. A "Cc: stable" on that patch shouldn't trigger

A patch like that should *NEVER* get CC:stable.

> But if you want to add this patch to edac-urgent, then go ahead.

I want us to agree on the nomenclature so that there are no misunderstandings.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] EDAC/i10nm: Fix mdev reference leak in i10nm_get_ddr_munits()

[Luck, Tony]

On Tue, Sep 22, 2026 at 11:53:07AM -0700, Borislav Petkov wrote:
> On Tue, Sep 22, 2026 at 07:59:56AM -0700, Luck, Tony wrote:
> > The "Fixes:" tag is a matter of fact. The "Cc: stable" is just an
> > opinion.
> 
> Are you being serious here?
> 
> "CC: stable" is a matter of opinion?!?!
> 
> Tony, where are you getting this from? Please point me to the docs where we
> state that, please, because I have never read it before.

In Documentation/process/howto.rst

  - After two weeks a -rc1 kernel is released and the focus is on making the
    new kernel as rock solid as possible.  Most of the patches at this point
    should fix a regression.  Bugs that have always existed are not
    regressions, so only push these kinds of fixes if they are important.

and Documentation/process/2.Process.rst

   The merge window lasts for approximately two weeks.  At the end of this
   time, Linus Torvalds will declare that the window is closed and release the
   first of the "rc" kernels.  For the kernel which is destined to be 9.x,
   for example, the release which happens at the end of the merge window will
   be called 9.x-rc1.  The -rc1 release is the signal that the time to
   merge new features has passed, and that the time to stabilize the next
   kernel has begun.

and Documentation/process/stable-kernel-rules.rst says when to add Cc: stable

  - It fixes a problem like an oops, a hang, data corruption, a real security
    issue, a hardware quirk, a build error (but not for things marked
    CONFIG_BROKEN), or some "oh, that's not good" issue.

> 
> Maybe I have missed that memo, who knows?
> 
> All I know is that stable folks appreciate and more or less rely on CC:stable
> from the patch committers so that they can know to pick up the patch quick. So
> CC:stable is actually a statement: this patch should go to stable.
> 
> Fixes: is just a tag that shows which commit it fixes.
>  
> > An extreme example would be a patch that fixes a spelling error in a
> > comment posted on a Friday before Linus is expected to tag a "final"
> > release in a cycle. A "Cc: stable" on that patch shouldn't trigger
> 
> A patch like that should *NEVER* get CC:stable.

The above is a clear example that should not be tagged for stable. I'm
suggesting that this EDAC bug doesn't meet the stable criteria.

I ought to have quizzed the submitter on why they thought it needed the
tag.
> 
> > But if you want to add this patch to edac-urgent, then go ahead.
> 
> I want us to agree on the nomenclature so that there are no misunderstandings.

-Tony