Add checks for ioremap return values in saa7164_dev_setup(). If
ioremap for BAR0 or BAR2 fails, release the already allocated PCI
memory regions, remove the device from the global list, decrement
the device count, and return -ENODEV.

This prevents potential null pointer dereferences and ensures proper
cleanup on memory mapping failures.

Signed-off-by: Wang Jun <1742789905@qq.com>
---
 drivers/media/pci/saa7164/saa7164-core.c | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/media/pci/saa7164/saa7164-core.c b/drivers/media/pci/saa7164/saa7164-core.c
index a8a004f28ca0..dc68d7cac7cf 100644
--- a/drivers/media/pci/saa7164/saa7164-core.c
+++ b/drivers/media/pci/saa7164/saa7164-core.c
@@ -998,9 +998,21 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
 	/* PCI/e allocations */
 	dev->lmmio = ioremap(pci_resource_start(dev->pci, 0),
 			     pci_resource_len(dev->pci, 0));
+	if (!dev->lmmio) {
+		dev_err(&dev->pci->dev,
+				"failed to remap MMIO memory @ 0x%llx\n",
+			(u64)pci_resource_start(dev->pci, 0));
+		goto err_ioremap;
+	}
 
 	dev->lmmio2 = ioremap(pci_resource_start(dev->pci, 2),
 			     pci_resource_len(dev->pci, 2));
+	if (!dev->lmmio2) {
+		dev_err(&dev->pci->dev,
+				"failed to remap MMIO memory @ 0x%llx\n",
+			(u64)pci_resource_start(dev->pci, 2));
+		goto err_ioremap2;
+	}
 
 	dev->bmmio = (u8 __iomem *)dev->lmmio;
 	dev->bmmio2 = (u8 __iomem *)dev->lmmio2;
@@ -1019,6 +1031,23 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
 	saa7164_pci_quirks(dev);
 
 	return 0;
+
+err_ioremap2:
+	iounmap(dev->lmmio);
+err_ioremap:
+	/* Release the PCI memory regions allocated in get_resources() */
+	release_mem_region(pci_resource_start(dev->pci, 0),
+					   pci_resource_len(dev->pci, 0));
+	release_mem_region(pci_resource_start(dev->pci, 2),
+					   pci_resource_len(dev->pci, 2));
+
+	/* Remove from device list and decrement count */
+	mutex_lock(&devlist);
+	list_del(&dev->devlist);
+	mutex_unlock(&devlist);
+	saa7164_devcount--;
+
+	return -ENODEV;
 }
 
 static void saa7164_dev_unregister(struct saa7164_dev *dev)
-- 
2.43.0

> Add checks for ioremap return values in saa7164_dev_setup(). If
> ioremap for BAR0 or BAR2 fails, release the already allocated PCI
> memory regions, remove the device from the global list, decrement
> the device count, and return -ENODEV.

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.0-rc3#n659


> This prevents potential null pointer dereferences and ensures proper
> cleanup on memory mapping failures.

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.0-rc3#n145


…
> +++ b/drivers/media/pci/saa7164/saa7164-core.c
> @@ -998,9 +998,21 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
>  	/* PCI/e allocations */
>  	dev->lmmio = ioremap(pci_resource_start(dev->pci, 0),
>  			     pci_resource_len(dev->pci, 0));
> +	if (!dev->lmmio) {
> +		dev_err(&dev->pci->dev,
> +				"failed to remap MMIO memory @ 0x%llx\n",
> +			(u64)pci_resource_start(dev->pci, 0));
> +		goto err_ioremap;
> +	}
>  
>  	dev->lmmio2 = ioremap(pci_resource_start(dev->pci, 2),
>  			     pci_resource_len(dev->pci, 2));
…

Would you like to avoid duplicate source code here?

…
> @@ -1019,6 +1031,23 @@ static int saa7164_dev_setup(struct saa7164_dev *dev)
>  	saa7164_pci_quirks(dev);
>  
>  	return 0;
…
> +	/* Remove from device list and decrement count */
> +	mutex_lock(&devlist);
> +	list_del(&dev->devlist);
> +	mutex_unlock(&devlist);
> +	saa7164_devcount--;
…

Will development interests grow to apply a call like “scoped_guard(mutex, &devlist)”?
https://elixir.bootlin.com/linux/v7.0-rc3/source/include/linux/mutex.h#L253

Regards,
Markus

Hi Markus,

This v4 addressed all your comments:
- Added `Cc: stable@vger.kernel.org` to 
ensure the fix reaches stable kernels.
- Refactored the duplicate ioremap error handling into a helper function 
to improve code clarity and reduce duplication.
- Replaced manual mutex lock/unlock with `scoped_guard()` for better 
safety and readability.

After submitting v4 (which became v5 in the patchwork series due to 
a version bump), the Media CI robot detected two issues:
- Build errors caused by incorrect struct member names 
(`immio` → `lmmio`, `pci_dev` → `pci`).
- Two checkpatch `CHECK` warnings about alignment of function call
 arguments (open parenthesis alignment).

This v6 fixes those issues:
- Corrected the member names to match the actual struct definitions.
- Adjusted the code alignment in the `release_resources()` helper to
 satisfy `checkpatch.pl --strict`.

This v7 addresses two additional issues that were identified 
in the v6 submission:
Corrected a remaining instance where dev->lmmio was used 
instead of the correct dev->lmmio2 in the foo_bar() function.

Regarding the `Fixes` tag: I've added a tag pointing to the 
commit that originally introduced the driver. 
At that time, the file
was located at drivers/media/video/saa7164/saa7164-core.c; the `Fixes` tag
correctly identifies the commit where the issue first appeared, regardless
of the later file move.

The patch adds missing error checks for two ioremap calls in
saa7164_dev_setup(). If either mapping fails, the function now properly
releases previously allocated PCI resources, removes the device from the
global list, and returns -ENODEV. This prevents potential null pointer
dereferences and ensures proper cleanup on failure.

Please review the updated patch. Thanks!

Wang Jun (1):
  media: saa7164: add ioremap return checks and cleanups

 drivers/media/pci/saa7164/saa7164-core.c | 47 ++++++++++++++++++------
 1 file changed, 35 insertions(+), 12 deletions(-)

-- 
2.43.0

Hi Markus,

This v4 addressed all your comments:
- Added `Cc: stable@vger.kernel.org` to 
ensure the fix reaches stable kernels.
- Refactored the duplicate ioremap error handling into a helper function 
to improve code clarity and reduce duplication.
- Replaced manual mutex lock/unlock with `scoped_guard()` for better 
safety and readability.

After submitting v4 (which became v5 in the patchwork series due to 
a version bump), the Media CI robot detected two issues:
- Build errors caused by incorrect struct member names 
(`immio` → `lmmio`, `pci_dev` → `pci`).
- Two checkpatch `CHECK` warnings about alignment of function call
 arguments (open parenthesis alignment).

This **v6** fixes those issues:
- Corrected the member names to match the actual struct definitions.
- Adjusted the code alignment in the `release_resources()` helper to
 satisfy `checkpatch.pl --strict`.

Regarding the `Fixes` tag: I've added a tag pointing to the 
commit that originally introduced the driver. 
At that time, the file
was located at drivers/media/video/saa7164/saa7164-core.c; the `Fixes` tag
correctly identifies the commit where the issue first appeared, regardless
of the later file move.

The patch adds missing error checks for two ioremap calls in
saa7164_dev_setup(). If either mapping fails, the function now properly
releases previously allocated PCI resources, removes the device from the
global list, and returns -ENODEV. This prevents potential null pointer
dereferences and ensures proper cleanup on failure.

Please review the updated patch. Thanks!

Wang Jun (1):
  media: saa7164: add ioremap return checks and cleanups

 drivers/media/pci/saa7164/saa7164-core.c | 47 ++++++++++++++++++------
 1 file changed, 35 insertions(+), 12 deletions(-)

-- 
2.43.0

Hi Markus,

Thank you for your thorough review of v3. This v5 addresses the issues
reported by the Media CI robot after v4 was submitted:
- Fixed build errors caused by incorrect struct member names
  (`immio` → `lmmio`, `pci_dev` → `pci`).
- Adjusted code alignment to satisfy checkpatch.pl (two CHECK
  warnings about open parenthesis alignment).

This v4 addresses all your comments:
- Added `Cc: stable@vger.kernel.org` to ensure the fix reaches stable
  kernels.
- Refactored the duplicate ioremap error handling into a helper function 
  to improve code clarity and reduce duplication.
- Replaced manual mutex lock/unlock with `scoped_guard()` 
  for better safety and readability.

Regarding the `Fixes` tag: I've added a tag pointing to the 
commit that originally introduced the driver. 
At that time, the file
was located at drivers/media/video/saa7164/saa7164-core.c; the `Fixes` tag
correctly identifies the commit where the issue first appeared, regardless
of the later file move.

The patch adds missing error checks for two ioremap calls in
saa7164_dev_setup(). If either mapping fails, the function now properly
releases previously allocated PCI resources, removes the device from the
global list, and returns -ENODEV. This prevents potential null pointer
dereferences and ensures proper cleanup on failure.

Please review the updated patch. Thanks!

Wang Jun (1):
  media: saa7164: add ioremap return checks and cleanups

 drivers/media/pci/saa7164/saa7164-core.c | 47 ++++++++++++++++++------
 1 file changed, 35 insertions(+), 12 deletions(-)

-- 
2.43.0

Hi Markus,

Thank you for your thorough review of v3. This v5 addresses the issues
reported by the Media CI robot after v4 was submitted:
- Fixed build errors caused by incorrect struct member names
  (`immio` → `lmmio`, `pci_dev` → `pci`).
- Adjusted code alignment to satisfy checkpatch.pl (two CHECK
  warnings about open parenthesis alignment).

This v4 addresses all your comments:
- Added `Cc: stable@vger.kernel.org` to ensure the fix reaches stable
  kernels.
- Refactored the duplicate ioremap error handling into a helper function 
  to improve code clarity and reduce duplication.
- Replaced manual mutex lock/unlock with `scoped_guard()` 
  for better safety and readability.

Regarding the `Fixes` tag: I've added a tag pointing to the 
commit that originally introduced the driver. 
At that time, the file
was located at drivers/media/video/saa7164/saa7164-core.c; the `Fixes` tag
correctly identifies the commit where the issue first appeared, regardless
of the later file move.

The patch adds missing error checks for two ioremap calls in
saa7164_dev_setup(). If either mapping fails, the function now properly
releases previously allocated PCI resources, removes the device from the
global list, and returns -ENODEV. This prevents potential null pointer
dereferences and ensures proper cleanup on failure.

Please review the updated patch. Thanks!

Wang Jun (1):
  media: saa7164: add ioremap return checks and cleanups

 drivers/media/pci/saa7164/saa7164-core.c | 47 ++++++++++++++++++------
 1 file changed, 35 insertions(+), 12 deletions(-)

-- 
2.43.0

Hi Markus,

Thank you for your thorough review of v3. 
This v4 addresses all your comments:

- Added `Cc: stable@vger.kernel.org` to ensure the fix reaches stable
  kernels.
- Refactored the duplicate ioremap error handling into a helper function 
  to improve code clarity and reduce duplication.
- Replaced manual mutex lock/unlock with `scoped_guard()` 
  for better safety and readability.

Regarding the `Fixes` tag: I've added a tag pointing to the 
commit that originally introduced the driver. 
At that time, the file
was located at drivers/media/video/saa7164/saa7164-core.c; the `Fixes` tag
correctly identifies the commit where the issue first appeared, regardless
of the later file move.

The patch adds missing error checks for two ioremap calls in
saa7164_dev_setup(). If either mapping fails, the function now properly
releases previously allocated PCI resources, removes the device from the
global list, and returns -ENODEV. This prevents potential null pointer
dereferences and ensures proper cleanup on failure.

Please review the updated patch. Thanks!

Wang Jun (1):
  media: saa7164: add ioremap return checks and cleanups

 drivers/media/pci/saa7164/saa7164-core.c | 41 +++++++++++++++++++++---
 1 file changed, 37 insertions(+), 4 deletions(-)

-- 
2.43.0