[v2] mfd: ls2kbmc: A fix and a refactoring

[PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Andy Shevchenko 2 months, 3 weeks ago

The mixing of managed and non-managed resources may lead to possible
use-after-free bugs. In this driver the problematic part is the device
functionality that may just have gone behind the functions back, e.g.,
when interrupt is being served. Fix this by switching to managed resources
for PCI.

Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/mfd/ls2k-bmc-core.c b/drivers/mfd/ls2k-bmc-core.c
index 5f38514fa89e..1d8be9cdb3a8 100644
--- a/drivers/mfd/ls2k-bmc-core.c
+++ b/drivers/mfd/ls2k-bmc-core.c
@@ -464,25 +464,23 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	resource_size_t base;
 	int ret;
 
-	ret = pci_enable_device(dev);
+	ret = pcim_enable_device(dev);
 	if (ret)
 		return ret;
 
 	ddata = devm_kzalloc(&dev->dev, sizeof(*ddata), GFP_KERNEL);
-	if (!ddata) {
-		ret = -ENOMEM;
-		goto disable_pci;
-	}
+	if (!ddata)
+		return -ENOMEM;
 
 	ddata->dev = &dev->dev;
 
 	ret = ls2k_bmc_init(ddata);
 	if (ret)
-		goto disable_pci;
+		return ret;
 
 	ret = ls2k_bmc_parse_mode(dev, &pd);
 	if (ret)
-		goto disable_pci;
+		return ret;
 
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].platform_data = &pd;
 	ls2k_bmc_cells[LS2K_BMC_DISPLAY].pdata_size = sizeof(pd);
@@ -490,23 +488,12 @@ static int ls2k_bmc_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	/* Remove conflicting efifb device */
 	ret = aperture_remove_conflicting_devices(base, SZ_4M, "simple-framebuffer");
-	if (ret) {
-		dev_err(&dev->dev, "Failed to removed firmware framebuffers: %d\n", ret);
-		goto disable_pci;
-	}
+	if (ret)
+		return dev_err_probe(&dev->dev, ret, "Failed to remove firmware framebuffers\n");
 
 	return devm_mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
 				    ls2k_bmc_cells, ARRAY_SIZE(ls2k_bmc_cells),
 				    &dev->resource[0], 0, NULL);
-
-disable_pci:
-	pci_disable_device(dev);
-	return ret;
-}
-
-static void ls2k_bmc_remove(struct pci_dev *dev)
-{
-	pci_disable_device(dev);
 }
 
 static struct pci_device_id ls2k_bmc_devices[] = {
@@ -519,7 +506,6 @@ static struct pci_driver ls2k_bmc_driver = {
 	.name = "ls2k-bmc",
 	.id_table = ls2k_bmc_devices,
 	.probe = ls2k_bmc_probe,
-	.remove = ls2k_bmc_remove,
 };
 module_pci_driver(ls2k_bmc_driver);
 
-- 
2.50.1

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Lee Jones 2 months, 2 weeks ago

On Thu, 13 Nov 2025, Andy Shevchenko wrote:

> The mixing of managed and non-managed resources may lead to possible
> use-after-free bugs. In this driver the problematic part is the device
> functionality that may just have gone behind the functions back, e.g.,
> when interrupt is being served. Fix this by switching to managed resources
> for PCI.
> 
> Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
>  1 file changed, 7 insertions(+), 21 deletions(-)

Still doesn't apply.  I'm getting lots of conflicts.

What base are you on?

% git --no-pager log --oneline drivers/mfd/ls2k-bmc-core.c
3696ac1d0db2 mfd: ls2kbmc: Remove unneeded semicolon from ls2k_bmc_recover_pci_data()
d952bba3fbb5 mfd: ls2kbmc: Add Loongson-2K BMC reset function support
0d64f6d1ffe9 mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Andy Shevchenko 2 months, 2 weeks ago

On Wed, Nov 19, 2025 at 04:47:03PM +0000, Lee Jones wrote:
> On Thu, 13 Nov 2025, Andy Shevchenko wrote:
> 
> > The mixing of managed and non-managed resources may lead to possible
> > use-after-free bugs. In this driver the problematic part is the device
> > functionality that may just have gone behind the functions back, e.g.,
> > when interrupt is being served. Fix this by switching to managed resources
> > for PCI.
> > 
> > Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> > Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> > Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
> >  1 file changed, 7 insertions(+), 21 deletions(-)
> 
> Still doesn't apply.  I'm getting lots of conflicts.
> 
> What base are you on?

Linux Next which includes your -fixes branch. You probably need to merge either
it or v6.18-rc6 to the -next. Then the v1 will be okay to apply.

The bottom line is that without doing that we will have conflicts either in
Linux Next followed by merging by Linus or in your branches locally before
going to the above mentioned.

Another possibility is cherry-pick patches from -fixes to -next.

And alternative is to wait for -rc1 and rebase this on top of and
apply then.

> % git --no-pager log --oneline drivers/mfd/ls2k-bmc-core.c
> 3696ac1d0db2 mfd: ls2kbmc: Remove unneeded semicolon from ls2k_bmc_recover_pci_data()
> d952bba3fbb5 mfd: ls2kbmc: Add Loongson-2K BMC reset function support
> 0d64f6d1ffe9 mfd: ls2kbmc: Introduce Loongson-2K BMC core driver

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Lee Jones 2 months, 2 weeks ago

On Wed, 19 Nov 2025, Andy Shevchenko wrote:

> On Wed, Nov 19, 2025 at 04:47:03PM +0000, Lee Jones wrote:
> > On Thu, 13 Nov 2025, Andy Shevchenko wrote:
> > 
> > > The mixing of managed and non-managed resources may lead to possible
> > > use-after-free bugs. In this driver the problematic part is the device
> > > functionality that may just have gone behind the functions back, e.g.,
> > > when interrupt is being served. Fix this by switching to managed resources
> > > for PCI.
> > > 
> > > Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> > > Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> > > Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
> > >  1 file changed, 7 insertions(+), 21 deletions(-)
> > 
> > Still doesn't apply.  I'm getting lots of conflicts.
> > 
> > What base are you on?
> 
> Linux Next which includes your -fixes branch. You probably need to merge either
> it or v6.18-rc6 to the -next. Then the v1 will be okay to apply.

Ah, yes, that explains it.

> The bottom line is that without doing that we will have conflicts either in
> Linux Next followed by merging by Linus or in your branches locally before
> going to the above mentioned.
> 
> Another possibility is cherry-pick patches from -fixes to -next.
> 
> And alternative is to wait for -rc1 and rebase this on top of and
> apply then.

Yes, if there is no urgency, we can wait for the next merge window.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Andy Shevchenko 2 months, 2 weeks ago

On Thu, Nov 20, 2025 at 08:54:02AM +0000, Lee Jones wrote:
> On Wed, 19 Nov 2025, Andy Shevchenko wrote:
> > On Wed, Nov 19, 2025 at 04:47:03PM +0000, Lee Jones wrote:
> > > On Thu, 13 Nov 2025, Andy Shevchenko wrote:

...

> > > > Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> > > > Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> > > > Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > ---
> > > >  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
> > > >  1 file changed, 7 insertions(+), 21 deletions(-)
> > > 
> > > Still doesn't apply.  I'm getting lots of conflicts.
> > > 
> > > What base are you on?
> > 
> > Linux Next which includes your -fixes branch. You probably need to merge either
> > it or v6.18-rc6 to the -next. Then the v1 will be okay to apply.
> 
> Ah, yes, that explains it.
> 
> > The bottom line is that without doing that we will have conflicts either in
> > Linux Next followed by merging by Linus or in your branches locally before
> > going to the above mentioned.
> > 
> > Another possibility is cherry-pick patches from -fixes to -next.
> > 
> > And alternative is to wait for -rc1 and rebase this on top of and
> > apply then.

> Yes, if there is no urgency,

It's up to you and the author of the code, it's a fix, but I don't think it's
too critical, we can wait, yes.

> we can wait for the next merge window.

It's a fix, do you mean to apply after -rc1 for v6.19-rcX?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Lee Jones 2 months, 2 weeks ago

On Thu, 20 Nov 2025, Andy Shevchenko wrote:

> On Thu, Nov 20, 2025 at 08:54:02AM +0000, Lee Jones wrote:
> > On Wed, 19 Nov 2025, Andy Shevchenko wrote:
> > > On Wed, Nov 19, 2025 at 04:47:03PM +0000, Lee Jones wrote:
> > > > On Thu, 13 Nov 2025, Andy Shevchenko wrote:
> 
> ...
> 
> > > > > Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> > > > > Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> > > > > Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > ---
> > > > >  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
> > > > >  1 file changed, 7 insertions(+), 21 deletions(-)
> > > > 
> > > > Still doesn't apply.  I'm getting lots of conflicts.
> > > > 
> > > > What base are you on?
> > > 
> > > Linux Next which includes your -fixes branch. You probably need to merge either
> > > it or v6.18-rc6 to the -next. Then the v1 will be okay to apply.
> > 
> > Ah, yes, that explains it.
> > 
> > > The bottom line is that without doing that we will have conflicts either in
> > > Linux Next followed by merging by Linus or in your branches locally before
> > > going to the above mentioned.
> > > 
> > > Another possibility is cherry-pick patches from -fixes to -next.
> > > 
> > > And alternative is to wait for -rc1 and rebase this on top of and
> > > apply then.
> 
> > Yes, if there is no urgency,
> 
> It's up to you and the author of the code, it's a fix, but I don't think it's
> too critical, we can wait, yes.
> 
> > we can wait for the next merge window.
> 
> It's a fix, do you mean to apply after -rc1 for v6.19-rcX?

Yes, we can merged it though for-mfd-fixes.

-- 
Lee Jones [李琼斯]

Re: [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Andy Shevchenko 2 months, 2 weeks ago

On Thu, Nov 20, 2025 at 01:39:27PM +0000, Lee Jones wrote:
> On Thu, 20 Nov 2025, Andy Shevchenko wrote:
> > On Thu, Nov 20, 2025 at 08:54:02AM +0000, Lee Jones wrote:
> > > On Wed, 19 Nov 2025, Andy Shevchenko wrote:
> > > > On Wed, Nov 19, 2025 at 04:47:03PM +0000, Lee Jones wrote:
> > > > > On Thu, 13 Nov 2025, Andy Shevchenko wrote:

...

> > > > > > Fixes: 91a3e1f5453a ("mfd: ls2kbmc: Check for devm_mfd_add_devices() failure")
> > > > > > Fixes: d952bba3fbb5 ("mfd: ls2kbmc: Add Loongson-2K BMC reset function support")
> > > > > > Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > > ---
> > > > > >  drivers/mfd/ls2k-bmc-core.c | 28 +++++++---------------------
> > > > > >  1 file changed, 7 insertions(+), 21 deletions(-)
> > > > > 
> > > > > Still doesn't apply.  I'm getting lots of conflicts.
> > > > > 
> > > > > What base are you on?
> > > > 
> > > > Linux Next which includes your -fixes branch. You probably need to merge either
> > > > it or v6.18-rc6 to the -next. Then the v1 will be okay to apply.
> > > 
> > > Ah, yes, that explains it.
> > > 
> > > > The bottom line is that without doing that we will have conflicts either in
> > > > Linux Next followed by merging by Linus or in your branches locally before
> > > > going to the above mentioned.
> > > > 
> > > > Another possibility is cherry-pick patches from -fixes to -next.
> > > > 
> > > > And alternative is to wait for -rc1 and rebase this on top of and
> > > > apply then.
> > 
> > > Yes, if there is no urgency,
> > 
> > It's up to you and the author of the code, it's a fix, but I don't think it's
> > too critical, we can wait, yes.
> > 
> > > we can wait for the next merge window.
> > 
> > It's a fix, do you mean to apply after -rc1 for v6.19-rcX?
> 
> Yes, we can merged it though for-mfd-fixes.

Okay, let's wait for v6.19-rc1 then.

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v2 1/2] mfd: ls2kbmc: Fully convert to use managed resources

Posted by Lee Jones 1 month, 3 weeks ago

On Thu, 13 Nov 2025 17:22:50 +0100, Andy Shevchenko wrote:
> The mixing of managed and non-managed resources may lead to possible
> use-after-free bugs. In this driver the problematic part is the device
> functionality that may just have gone behind the functions back, e.g.,
> when interrupt is being served. Fix this by switching to managed resources
> for PCI.
> 
> 
> [...]

Applied, thanks!

[1/2] mfd: ls2kbmc: Fully convert to use managed resources
      commit: 5bf5a793afc97fad001864d4268a45d355703956

--
Lee Jones [李琼斯]