[PATCH] soc: fsl: qe: Fix potential NULL pointer dereference in qe_reset()

[PATCH] soc: fsl: qe: Fix potential NULL pointer dereference in qe_reset()

[Wang Jun]

The function qe_reset() uses qe_immr without checking if it is NULL,
which could happen if ioremap() failed earlier. Add a NULL check and
perform ioremap() if needed; if it still fails, print an error and
return to avoid crashing the system.

Signed-off-by: Wang Jun <1742789905@qq.com>
---
 drivers/soc/fsl/qe/qe.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 70b6eddb867b..6dcfa340970a 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -86,8 +86,13 @@ static phys_addr_t get_qe_base(void)
 
 void qe_reset(void)
 {
-	if (qe_immr == NULL)
+	if (qe_immr == NULL) {
 		qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
+		if (qe_immr == NULL) {
+			pr_err("QE: cannot remap IMMR\n");
+			return;
+		}
+	}
 
 	qe_snums_init();
 
-- 
2.43.0

Re: [PATCH] soc: fsl: qe: Fix potential NULL pointer dereference in qe_reset()

[Christophe Leroy (CS GROUP)]

Le 10/03/2026 à 13:11, Wang Jun a écrit :
> [Vous ne recevez pas souvent de courriers de 1742789905@qq.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> The function qe_reset() uses qe_immr without checking if it is NULL,
> which could happen if ioremap() failed earlier. Add a NULL check and
> perform ioremap() if needed; if it still fails, print an error and
> return to avoid crashing the system.

I don't understand what you are trying to say here. What you say is 
already what qe_reset() does: it does a NULL check and performs 
ioremap() when it is NULL:

	if (qe_immr == NULL)
		qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);

You are adding a second NULL check and return early from qe_reset(). But 
it doesn't really fix the problem because qe_immr is used in many other 
places so you are just delaying the problem.

What needs to be done is that if qe_immr remap fails, all drivers 
depending on it don't get probed.

> 
> Signed-off-by: Wang Jun <1742789905@qq.com>
> ---
>   drivers/soc/fsl/qe/qe.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index 70b6eddb867b..6dcfa340970a 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -86,8 +86,13 @@ static phys_addr_t get_qe_base(void)
> 
>   void qe_reset(void)
>   {
> -       if (qe_immr == NULL)
> +       if (qe_immr == NULL) {
>                  qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
> +               if (qe_immr == NULL) {
> +                       pr_err("QE: cannot remap IMMR\n");
> +                       return;
> +               }
> +       }
> 
>          qe_snums_init();
> 
> --
> 2.43.0
> 

Re: [PATCH] soc: fsl: qe: Fix potential NULL pointer dereference inqe_reset()

[Christophe Leroy (CS GROUP)]

Hi,

Le 16/03/2026 à 04:28, 未君 a écrit :
> 	
> Vous n’obtenez pas souvent d’e-mail à partir de 1742789905@qq.com. 
> Pourquoi c’est important <https://aka.ms/LearnAboutSenderIdentification>
> 	
> 
> Hi,
> 
> Thank you for the detailed review. You are completely right.
> 
> My commit message was confusing, and returning early in qe_reset() just 
> shifts the NULL pointer dereference to the dependent drivers later on, 
> without actually fixing the root cause.
> 
> To achieve what you suggested ("if qe_immr remap fails, all drivers 
> depending on it don't get probed"), I plan to do the following in the v2 
> patch:
> 
> 1. Change the return type of qe_reset() from `void` to `int`.
> 2. Return `-ENOMEM` if the ioremap() fails.
> 3. Update the callers of qe_reset() (e.g., qe_probe() and other board- 
> specific setup functions) to check this return value. If qe_reset() 
> fails, the callers will abort their initialization/probing, which will 
> properly prevent the child devices from being probed.
> 
> Does this approach sound correct to you? If so, I will prepare and 
> submit the v2 patch accordingly.

Well, it would probably work but is it worth it ?

If the board is already unable to get a few bytes of memory that early 
in the boot process it is unlikely that it will be able to do much more 
work.

Wouldn't it be good enough to just panic() when ioremap() fails, similar 
to what happens when qe_sdma_init() fails, see 
https://elixir.bootlin.com/linux/v7.0-rc5/source/drivers/soc/fsl/qe/qe.c#L101

Christophe

[PATCH v2] soc: fsl: qe: panic on ioremap() failure in qe_reset()

[Wang Jun]

When ioremap() fails in qe_reset(), the global pointer qe_immr remains
NULL, leading to a subsequent NULL pointer dereference when the pointer
is accessed. Since this happens early in the boot process, a failure to
map a few bytes of I/O memory indicates a fatal error from which the
system cannot recover.

Follow the same pattern as qe_sdma_init() and panic immediately when
ioremap() fails. This avoids a silent NULL pointer dereference later
and makes the error explicit.

Fixes: 986585385131 ("[POWERPC] Add QUICC Engine (QE) infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Wang Jun <1742789905@qq.com>
---
 drivers/soc/fsl/qe/qe.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 70b6eddb867b..9f6223043ee3 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -86,8 +86,12 @@ static phys_addr_t get_qe_base(void)
 
 void qe_reset(void)
 {
-	if (qe_immr == NULL)
+	if (qe_immr == NULL) {
 		qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
+		if (qe_immr == NULL) {
+			panic("QE:ioremap failed!");
+		}
+	}
 
 	qe_snums_init();
 
-- 
2.43.0

Re: [PATCH v2] soc: fsl: qe: panic on ioremap() failure in qe_reset()

[Christophe Leroy (CS GROUP)]

Le 27/03/2026 à 01:12, Wang Jun a écrit :
> [Vous ne recevez pas souvent de courriers de 1742789905@qq.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> 
> When ioremap() fails in qe_reset(), the global pointer qe_immr remains
> NULL, leading to a subsequent NULL pointer dereference when the pointer
> is accessed. Since this happens early in the boot process, a failure to
> map a few bytes of I/O memory indicates a fatal error from which the
> system cannot recover.
> 
> Follow the same pattern as qe_sdma_init() and panic immediately when
> ioremap() fails. This avoids a silent NULL pointer dereference later
> and makes the error explicit.
> 
> Fixes: 986585385131 ("[POWERPC] Add QUICC Engine (QE) infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wang Jun <1742789905@qq.com>
> ---
>   drivers/soc/fsl/qe/qe.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
> index 70b6eddb867b..9f6223043ee3 100644
> --- a/drivers/soc/fsl/qe/qe.c
> +++ b/drivers/soc/fsl/qe/qe.c
> @@ -86,8 +86,12 @@ static phys_addr_t get_qe_base(void)
> 
>   void qe_reset(void)
>   {
> -       if (qe_immr == NULL)
> +       if (qe_immr == NULL) {
>                  qe_immr = ioremap(get_qe_base(), QE_IMMAP_SIZE);
> +               if (qe_immr == NULL) {
> +                       panic("QE:ioremap failed!");
> +               }
> +       }
> 
>          qe_snums_init();
> 

Applied, thanks.