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

[未君]

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.


Best regards,
Wang Jun


未君
1742789905 at qq.com



        



         Original
         
       
From: Christophe Leroy (CS GROUP) <chleroy at kernel.org>
Date: 2026-03-13 17:48
To: Wang Jun <1742789905 at qq.com>, Qiang Zhao <qiang.zhao at nxp.com>, linuxppc-dev <linuxppc-dev at lists.ozlabs.org>, linux-arm-kernel <linux-arm-kernel at lists.infradead.org>
Cc: linux-kernel <linux-kernel at vger.kernel.org>, gszhai <gszhai at bjtu.edu.cn>, 25125332 <25125332 at bjtu.edu.cn>, 25125283 <25125283 at bjtu.edu.cn>, 23120469 <23120469 at bjtu.edu.cn>
Subject: Re: [PATCH] soc: fsl: qe: Fix potential NULL pointer dereference inqe_reset()





Le 10/03/2026 à 13:11, Wang Jun a écrit :
> [Vous ne recevez pas souvent de courriers de 1742789905 at 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 at 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
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20260316/304d3720/attachment.htm>