[PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free

[PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free

[Gou Hao]

Changes in v2:
- Remove the now-unused out_free_domain error label in the xive
  patch after deleting the unreachable NULL check goto.
- Fix a use-after-free of xive_ipis: when xive_init_ipis() fails,
  the error path frees xive_ipis but xive_smp_probe() ignores the
  error and proceeds to dereference it in xive_setup_cpu_ipi().
  Propagate the error return through xive_smp_probe() and check
  it in both pnv_smp_probe() and pSeries_smp_probe().

This is a code cleanup series that removes unreachable
NULL checks (and associated error handling) that follow allocations
using __GFP_NOFAIL, which guarantees non-NULL return.

Gou Hao (6):
  powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation
  powerpc/xive: add error return value to xive_smp_probe()
  powerpc/xive: fix use-after-free of xive_ipis
  drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation
  lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations
  RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations

 arch/powerpc/include/asm/xive.h      |  4 ++--
 arch/powerpc/platforms/powernv/smp.c |  8 +++++---
 arch/powerpc/platforms/pseries/smp.c |  8 +++++---
 arch/powerpc/sysdev/xive/common.c    | 15 +++++++++------
 drivers/gpu/drm/drm_modeset_lock.c   |  4 ----
 drivers/infiniband/hw/cxgb4/mem.c    | 10 ++--------
 lib/test_hmm.c                       |  6 ------
 7 files changed, 23 insertions(+), 32 deletions(-)

-- 
2.20.1

[PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation

[Gou Hao]

kzalloc_objs with the __GFP_NOFAIL flag will never return NULL, so the
subsequent NULL check is unreachable dead code. Remove it.

Signed-off-by: Gou Hao <gouhao@uniontech.com>
Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
---
 arch/powerpc/sysdev/xive/common.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index dadd1f46ec939..f2904a5c2b7bf 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1134,9 +1134,6 @@ static int __init xive_init_ipis(void)
 
 	xive_ipis = kzalloc_objs(*xive_ipis, nr_node_ids,
 				 GFP_KERNEL | __GFP_NOFAIL);
-	if (!xive_ipis)
-		goto out_free_domain;
-
 	for_each_node(node) {
 		struct xive_ipi_desc *xid = &xive_ipis[node];
 		struct xive_ipi_alloc_info info = { node };
@@ -1158,7 +1155,7 @@ static int __init xive_init_ipis(void)
 
 out_free_xive_ipis:
 	kfree(xive_ipis);
-out_free_domain:
+	xive_ipis = NULL;
 	irq_domain_remove(ipi_domain);
 out_free_fwnode:
 	irq_domain_free_fwnode(fwnode);
-- 
2.20.1

Re: [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation

[Cédric Le Goater]

On 7/24/26 04:28, Gou Hao wrote:
> kzalloc_objs with the __GFP_NOFAIL flag will never return NULL, so the
> subsequent NULL check is unreachable dead code. Remove it.
> 
> Signed-off-by: Gou Hao <gouhao@uniontech.com>
> Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
> Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
> ---
>   arch/powerpc/sysdev/xive/common.c | 5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index dadd1f46ec939..f2904a5c2b7bf 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1134,9 +1134,6 @@ static int __init xive_init_ipis(void)
>   
>   	xive_ipis = kzalloc_objs(*xive_ipis, nr_node_ids,
>   				 GFP_KERNEL | __GFP_NOFAIL);
> -	if (!xive_ipis)
> -		goto out_free_domain;
> -
>   	for_each_node(node) {
>   		struct xive_ipi_desc *xid = &xive_ipis[node];
>   		struct xive_ipi_alloc_info info = { node };
> @@ -1158,7 +1155,7 @@ static int __init xive_init_ipis(void)
>   
>   out_free_xive_ipis:
>   	kfree(xive_ipis);
> -out_free_domain:
> +	xive_ipis = NULL;
>   	irq_domain_remove(ipi_domain);
>   out_free_fwnode:
>   	irq_domain_free_fwnode(fwnode);

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

[PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe()

[Gou Hao]

xive_smp_probe() calls xive_init_ipis() which can fail, but its
return value is currently ignored.  Change xive_smp_probe() to
return int so that errors can be propagated to callers.

This is a preparatory patch for the next one.

No functional change yet; the return value is always 0 at this point.

Signed-off-by: Gou Hao <gouhao@uniontech.com>
Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
---
 arch/powerpc/include/asm/xive.h   | 4 ++--
 arch/powerpc/sysdev/xive/common.c | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
index efb0f5effcc69..4e3e3358993c0 100644
--- a/arch/powerpc/include/asm/xive.h
+++ b/arch/powerpc/include/asm/xive.h
@@ -91,7 +91,7 @@ static inline bool xive_enabled(void) { return __xive_enabled; }
 
 bool xive_spapr_init(void);
 bool xive_native_init(void);
-void xive_smp_probe(void);
+int xive_smp_probe(void);
 int  xive_smp_prepare_cpu(unsigned int cpu);
 void xive_smp_setup_cpu(void);
 void xive_smp_disable_cpu(void);
@@ -153,7 +153,7 @@ static inline bool xive_enabled(void) { return false; }
 
 static inline bool xive_spapr_init(void) { return false; }
 static inline bool xive_native_init(void) { return false; }
-static inline void xive_smp_probe(void) { }
+static inline int xive_smp_probe(void) { return -EINVAL; }
 static inline int  xive_smp_prepare_cpu(unsigned int cpu) { return -EINVAL; }
 static inline void xive_smp_setup_cpu(void) { }
 static inline void xive_smp_disable_cpu(void) { }
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index f2904a5c2b7bf..f9a1229cede73 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1254,7 +1254,7 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
 	xive_ops->put_ipi(cpu, xc);
 }
 
-void __init xive_smp_probe(void)
+int __init xive_smp_probe(void)
 {
 	smp_ops->cause_ipi = xive_cause_ipi;
 
@@ -1263,6 +1263,8 @@ void __init xive_smp_probe(void)
 
 	/* Allocate and setup IPI for the boot CPU */
 	xive_setup_cpu_ipi(smp_processor_id());
+
+	return 0;
 }
 
 #endif /* CONFIG_SMP */
-- 
2.20.1

Re: [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe()

[Cédric Le Goater]

On 7/24/26 04:28, Gou Hao wrote:
> xive_smp_probe() calls xive_init_ipis() which can fail, but its
> return value is currently ignored.  Change xive_smp_probe() to
> return int so that errors can be propagated to callers.
> 
> This is a preparatory patch for the next one.
> 
> No functional change yet; the return value is always 0 at this point.
> 
> Signed-off-by: Gou Hao <gouhao@uniontech.com>
> Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
> Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
> ---
>   arch/powerpc/include/asm/xive.h   | 4 ++--
>   arch/powerpc/sysdev/xive/common.c | 4 +++-
>   2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/xive.h b/arch/powerpc/include/asm/xive.h
> index efb0f5effcc69..4e3e3358993c0 100644
> --- a/arch/powerpc/include/asm/xive.h
> +++ b/arch/powerpc/include/asm/xive.h
> @@ -91,7 +91,7 @@ static inline bool xive_enabled(void) { return __xive_enabled; }
>   
>   bool xive_spapr_init(void);
>   bool xive_native_init(void);
> -void xive_smp_probe(void);
> +int xive_smp_probe(void);
>   int  xive_smp_prepare_cpu(unsigned int cpu);
>   void xive_smp_setup_cpu(void);
>   void xive_smp_disable_cpu(void);
> @@ -153,7 +153,7 @@ static inline bool xive_enabled(void) { return false; }
>   
>   static inline bool xive_spapr_init(void) { return false; }
>   static inline bool xive_native_init(void) { return false; }
> -static inline void xive_smp_probe(void) { }
> +static inline int xive_smp_probe(void) { return -EINVAL; }
>   static inline int  xive_smp_prepare_cpu(unsigned int cpu) { return -EINVAL; }
>   static inline void xive_smp_setup_cpu(void) { }
>   static inline void xive_smp_disable_cpu(void) { }
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index f2904a5c2b7bf..f9a1229cede73 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1254,7 +1254,7 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
>   	xive_ops->put_ipi(cpu, xc);
>   }
>   
> -void __init xive_smp_probe(void)
> +int __init xive_smp_probe(void)
>   {
>   	smp_ops->cause_ipi = xive_cause_ipi;
>   
> @@ -1263,6 +1263,8 @@ void __init xive_smp_probe(void)
>   
>   	/* Allocate and setup IPI for the boot CPU */
>   	xive_setup_cpu_ipi(smp_processor_id());
> +
> +	return 0;
>   }
>   
>   #endif /* CONFIG_SMP */

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

[PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis

[Gou Hao]

When irq_domain_alloc_irqs() fails in xive_init_ipis(), the error
path frees the global xive_ipis array via kfree().  However,
xive_smp_probe() ignores the error return and proceeds to call
xive_setup_cpu_ipi(), which dereferences the already-freed xive_ipis
pointer, resulting in a use-after-free.

Propagate the error from xive_init_ipis() through xive_smp_probe()
and check it in both pnv_smp_probe() and pSeries_smp_probe() so that
IPI setup is aborted cleanly on failure.

Fixes: 7dcc37b3eff9 ("powerpc/xive: Map one IPI interrupt per node")
Signed-off-by: Gou Hao <gouhao@uniontech.com>
Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
---
 arch/powerpc/platforms/powernv/smp.c | 8 +++++---
 arch/powerpc/platforms/pseries/smp.c | 8 +++++---
 arch/powerpc/sysdev/xive/common.c    | 6 +++++-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
index 8f41ef364fc6f..b1201dbafcaf6 100644
--- a/arch/powerpc/platforms/powernv/smp.c
+++ b/arch/powerpc/platforms/powernv/smp.c
@@ -332,10 +332,12 @@ static void pnv_cause_ipi(int cpu)
 
 static void __init pnv_smp_probe(void)
 {
-	if (xive_enabled())
-		xive_smp_probe();
-	else
+	if (xive_enabled()) {
+		if (xive_smp_probe() < 0)
+			return;
+	} else {
 		xics_smp_probe();
+	}
 
 	if (cpu_has_feature(CPU_FTR_DBELL)) {
 		ic_cause_ipi = smp_ops->cause_ipi;
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index db99725e752bd..14cd0634eeca8 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -194,10 +194,12 @@ static int pseries_cause_nmi_ipi(int cpu)
 
 static __init void pSeries_smp_probe(void)
 {
-	if (xive_enabled())
-		xive_smp_probe();
-	else
+	if (xive_enabled()) {
+		if (xive_smp_probe() < 0)
+			return;
+	} else {
 		xics_smp_probe();
+	}
 
 	/* No doorbell facility, must use the interrupt controller for IPIs */
 	if (!cpu_has_feature(CPU_FTR_DBELL))
diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
index f9a1229cede73..bb6ce07c1699e 100644
--- a/arch/powerpc/sysdev/xive/common.c
+++ b/arch/powerpc/sysdev/xive/common.c
@@ -1256,10 +1256,14 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
 
 int __init xive_smp_probe(void)
 {
+	int ret;
+
 	smp_ops->cause_ipi = xive_cause_ipi;
 
 	/* Register the IPI */
-	xive_init_ipis();
+	ret = xive_init_ipis();
+	if (ret < 0)
+		return ret;
 
 	/* Allocate and setup IPI for the boot CPU */
 	xive_setup_cpu_ipi(smp_processor_id());
-- 
2.20.1

Re: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix: NULL pointer dereference on secondary CPU bring-up.
- [High] Missing error propagation from xive_setup_cpu_ipi().
--

--- Patch [3]: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis ---
commit da9c3cedc79737a7541cbdd8b690abe6862a200e
Author: Gou Hao <gouhao@uniontech.com>

powerpc/xive: fix use-after-free of xive_ipis

This commit propagates the error from xive_init_ipis() through
xive_smp_probe() to pnv_smp_probe() and pSeries_smp_probe(). It allows
the boot sequence to cleanly abort XIVE IPI setup if the initial allocation
fails.

> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index 8f41ef364fc6f..b1201dbafcaf6 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -332,10 +332,12 @@ static void pnv_cause_ipi(int cpu)
>  
>  static void __init pnv_smp_probe(void)
>  {
> -	if (xive_enabled())
> -		xive_smp_probe();
> -	else
> +	if (xive_enabled()) {
> +		if (xive_smp_probe() < 0)
> +			return;

[Severity: High]
This is a pre-existing issue, but does this early return leave XIVE
globally enabled if initialization fails?

Later during boot, SMP bring-up calls xive_smp_prepare_cpu() for secondary
CPUs because xive_enabled() remains true.

This unconditionally invokes xive_setup_cpu_ipi(), which calls
xive_ipi_cpu_to_irq() and dereferences the xive_ipis array pointer:

arch/powerpc/sysdev/xive/common.c:xive_ipi_cpu_to_irq()
    ...
    return xive_ipis[early_cpu_to_node(cpu)].irq;
    ...

Since xive_init_ipis() frees xive_ipis and sets it to NULL on failure,
could this cause a NULL pointer dereference on secondary CPU bring-up?

> +	} else {
>  		xics_smp_probe();
> +	}

[ ... ]

> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index f9a1229cede73..bb6ce07c1699e 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1256,10 +1256,14 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
>  
>  int __init xive_smp_probe(void)
>  {
> +	int ret;
> +
>  	smp_ops->cause_ipi = xive_cause_ipi;
>  
>  	/* Register the IPI */
> -	xive_init_ipis();
> +	ret = xive_init_ipis();
> +	if (ret < 0)
> +		return ret;
>  
>  	/* Allocate and setup IPI for the boot CPU */
>  	xive_setup_cpu_ipi(smp_processor_id());

[Severity: High]
This is a pre-existing issue, but shouldn't the return value of
xive_setup_cpu_ipi() be checked and propagated here?

If hardware IPI allocation or configuration fails (e.g., returning -EIO),
the error code is ignored and xive_smp_probe() returns 0 to its caller.

Could this lead to the boot process continuing with uninitialized hardware
IPIs on the boot CPU, eventually resulting in a system hang due to missing
cross-CPU interrupts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724022851.466017-1-gouhao@uniontech.com?part=3

Re: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis

[Cédric Le Goater]

On 7/24/26 04:28, Gou Hao wrote:
> When irq_domain_alloc_irqs() fails in xive_init_ipis(), the error
> path frees the global xive_ipis array via kfree().  However,
> xive_smp_probe() ignores the error return and proceeds to call
> xive_setup_cpu_ipi(), which dereferences the already-freed xive_ipis
> pointer, resulting in a use-after-free.
> 
> Propagate the error from xive_init_ipis() through xive_smp_probe()
> and check it in both pnv_smp_probe() and pSeries_smp_probe() so that
> IPI setup is aborted cleanly on failure.
> 
> Fixes: 7dcc37b3eff9 ("powerpc/xive: Map one IPI interrupt per node")
> Signed-off-by: Gou Hao <gouhao@uniontech.com>
> Reviewed-by: Wentao Guan <guanwentao@uniontech.com>
> Reviewed-by: jiazhenyuan <jiazhenyuan@uniontech.com>
> ---
>   arch/powerpc/platforms/powernv/smp.c | 8 +++++---
>   arch/powerpc/platforms/pseries/smp.c | 8 +++++---
>   arch/powerpc/sysdev/xive/common.c    | 6 +++++-
>   3 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/smp.c b/arch/powerpc/platforms/powernv/smp.c
> index 8f41ef364fc6f..b1201dbafcaf6 100644
> --- a/arch/powerpc/platforms/powernv/smp.c
> +++ b/arch/powerpc/platforms/powernv/smp.c
> @@ -332,10 +332,12 @@ static void pnv_cause_ipi(int cpu)
>   
>   static void __init pnv_smp_probe(void)
>   {
> -	if (xive_enabled())
> -		xive_smp_probe();
> -	else
> +	if (xive_enabled()) {
> +		if (xive_smp_probe() < 0)
> +			return;
> +	} else {
>   		xics_smp_probe();
> +	}
>   
>   	if (cpu_has_feature(CPU_FTR_DBELL)) {
>   		ic_cause_ipi = smp_ops->cause_ipi;
> diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
> index db99725e752bd..14cd0634eeca8 100644
> --- a/arch/powerpc/platforms/pseries/smp.c
> +++ b/arch/powerpc/platforms/pseries/smp.c
> @@ -194,10 +194,12 @@ static int pseries_cause_nmi_ipi(int cpu)
>   
>   static __init void pSeries_smp_probe(void)
>   {
> -	if (xive_enabled())
> -		xive_smp_probe();
> -	else
> +	if (xive_enabled()) {
> +		if (xive_smp_probe() < 0)
> +			return;
> +	} else {
>   		xics_smp_probe();
> +	}
>   
>   	/* No doorbell facility, must use the interrupt controller for IPIs */
>   	if (!cpu_has_feature(CPU_FTR_DBELL))
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index f9a1229cede73..bb6ce07c1699e 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1256,10 +1256,14 @@ noinstr static void xive_cleanup_cpu_ipi(unsigned int cpu, struct xive_cpu *xc)
>   
>   int __init xive_smp_probe(void)
>   {
> +	int ret;
> +
>   	smp_ops->cause_ipi = xive_cause_ipi;
>   
>   	/* Register the IPI */
> -	xive_init_ipis();
> +	ret = xive_init_ipis();
> +	if (ret < 0)
> +		return ret;
>   
>   	/* Allocate and setup IPI for the boot CPU */
>   	xive_setup_cpu_ipi(smp_processor_id());

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

[PATCH v2 4/6] drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation

[Gou Hao]

kzalloc_obj with the __GFP_NOFAIL flag will never return NULL, so the
subsequent WARN_ON(!ctx) is unreachable dead code. Remove it.

Signed-off-by: Gou Hao <gouhao@uniontech.com>
---
 drivers/gpu/drm/drm_modeset_lock.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 2c806b0146d67..e14814c30d8c0 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -149,11 +149,7 @@ void drm_modeset_lock_all(struct drm_device *dev)
 	int ret;
 
 	ctx = kzalloc_obj(*ctx, GFP_KERNEL | __GFP_NOFAIL);
-	if (WARN_ON(!ctx))
-		return;
-
 	mutex_lock(&config->mutex);
-
 	drm_modeset_acquire_init(ctx, 0);
 
 retry:
-- 
2.20.1

[PATCH v2 5/6] lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations

[Gou Hao]

kvcalloc with the __GFP_NOFAIL flag will never return NULL, so the
subsequent NULL checks are unreachable dead code. Remove them.

Signed-off-by: Gou Hao <gouhao@uniontech.com>
---
 lib/test_hmm.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/lib/test_hmm.c b/lib/test_hmm.c
index 9c59d1ceb5b57..d615e4e5fc447 100644
--- a/lib/test_hmm.c
+++ b/lib/test_hmm.c
@@ -1209,16 +1209,10 @@ static int dmirror_migrate_to_device(struct dmirror *dmirror,
 	if (!mmget_not_zero(mm))
 		return -EINVAL;
 
-	ret = -ENOMEM;
 	src_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*src_pfns),
 			  GFP_KERNEL | __GFP_NOFAIL);
-	if (!src_pfns)
-		goto free_mem;
-
 	dst_pfns = kvcalloc(PTRS_PER_PTE, sizeof(*dst_pfns),
 			  GFP_KERNEL | __GFP_NOFAIL);
-	if (!dst_pfns)
-		goto free_mem;
 
 	ret = 0;
 	mmap_read_lock(mm);
-- 
2.20.1

[PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations

[Gou Hao]

alloc_skb() with the __GFP_NOFAIL flag will never return NULL,
so the subsequent NULL checks and error handling are unreachable
dead code. Remove them.

Signed-off-by: Gou Hao <gouhao@uniontech.com>
---
 drivers/infiniband/hw/cxgb4/mem.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c
index cd1b010141984..08631de17c0a3 100644
--- a/drivers/infiniband/hw/cxgb4/mem.c
+++ b/drivers/infiniband/hw/cxgb4/mem.c
@@ -74,11 +74,8 @@ static int _c4iw_write_mem_dma_aligned(struct c4iw_rdev *rdev, u32 addr,
 		c4iw_init_wr_wait(wr_waitp);
 	wr_len = roundup(sizeof(*req) + sizeof(*sgl), 16);
 
-	if (!skb) {
+	if (!skb)
 		skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
-		if (!skb)
-			return -ENOMEM;
-	}
 	set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
 
 	req = __skb_put_zero(skb, wr_len);
@@ -134,11 +131,8 @@ static int _c4iw_write_mem_inline(struct c4iw_rdev *rdev, u32 addr, u32 len,
 					 roundup(copy_len, T4_ULPTX_MIN_IO),
 				 16);
 
-		if (!skb) {
+		if (!skb)
 			skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
-			if (!skb)
-				return -ENOMEM;
-		}
 		set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
 
 		req = __skb_put_zero(skb, wr_len);
-- 
2.20.1

Re: [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free of `mhp->dereg_skb` in MR allocation error paths (`c4iw_get_dma_mr`).
- [Medium] `write_adapter_mem()` passes an already freed skb to `_c4iw_write_mem_inline()` if `_c4iw_write_mem_dma()` fails.
- [Medium] Leaked `mhp->dereg_skb` in `c4iw_dereg_mr()` during device fatal error.
--

--- Patch [6]: [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations ---
commit 8aa6275ceca7a8b20e9109cfd6059d14769af15d
Author: Gou Hao <gouhao@uniontech.com>

RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations

alloc_skb() with the __GFP_NOFAIL flag will never return NULL,
so the subsequent NULL checks and error handling are unreachable
dead code. Remove them.

> diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c
> index cd1b010141984..08631de17c0a3 100644
> --- a/drivers/infiniband/hw/cxgb4/mem.c
> +++ b/drivers/infiniband/hw/cxgb4/mem.c
> @@ -74,11 +74,8 @@ static int _c4iw_write_mem_dma_aligned(struct c4iw_rdev *rdev, u32 addr,
>  		c4iw_init_wr_wait(wr_waitp);
>  	wr_len = roundup(sizeof(*req) + sizeof(*sgl), 16);
>  
> -	if (!skb) {
> +	if (!skb)
>  		skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
> -		if (!skb)
> -			return -ENOMEM;
> -	}
>  	set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
>  
>  	req = __skb_put_zero(skb, wr_len);
> @@ -134,11 +131,8 @@ static int _c4iw_write_mem_inline(struct c4iw_rdev *rdev, u32 addr, u32 len,
>  					 roundup(copy_len, T4_ULPTX_MIN_IO),
>  				 16);
>  
> -		if (!skb) {
> +		if (!skb)
>  			skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
> -			if (!skb)
> -				return -ENOMEM;
> -		}
>  		set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
>  
>  		req = __skb_put_zero(skb, wr_len);

[ ... ]

> static int write_adapter_mem(struct c4iw_rdev *rdev, u32 addr, u32 len,
> 			     void *data, struct sk_buff *skb,
> 			     struct c4iw_wr_wait *wr_waitp)
> {
[ ... ]
> 	ret = _c4iw_write_mem_dma(rdev, addr, len, data, skb, wr_waitp);
> 	if (ret) {
> 		pr_warn_ratelimited("%s: dma map failure (non fatal)\n",
> 				    pci_name(rdev->lldi.pdev));
> 		ret = _c4iw_write_mem_inline(rdev, addr, len, data, skb,
> 					      wr_waitp);
> 	}
> out:
> 	return ret;
> 
> }

[Severity: Medium]
This isn't a bug introduced by this patch, but does the error handling path
here pass an already freed skb to _c4iw_write_mem_inline()?

If _c4iw_write_mem_dma() encounters a transmission error (e.g., c4iw_ofld_send
fails), the network layer will consume and free the skb.

Does write_adapter_mem() incorrectly assume any error from the DMA function is
a non-fatal mapping error, and fall back to _c4iw_write_mem_inline() passing
the exact same freed skb pointer?

[ ... ]

> struct ib_mr *c4iw_get_dma_mr(struct ib_pd *pd, int acc)
> {
[ ... ]
> 	ret = write_tpt_entry(&rhp->rdev, 0, &stag, 1, php->pdid,
> 			      FW_RI_STAG_NSMR, mhp->attr.perms,
> 			      mhp->attr.mw_bind_enable, 0, 0, ~0ULL, 0, 0, 0,
> 			      NULL, mhp->wr_waitp);
> 	if (ret)
> 		goto err_free_skb;
> 
> 	ret = finish_mem_reg(mhp, stag);
> 	if (ret)
> 		goto err_dereg_mem;
> 	return &mhp->ibmr;
> err_dereg_mem:
> 	dereg_mem(&rhp->rdev, mhp->attr.stag, mhp->attr.pbl_size,
> 		  mhp->attr.pbl_addr, mhp->dereg_skb, mhp->wr_waitp);
> err_free_skb:
> 	kfree_skb(mhp->dereg_skb);
> err_free_wr_wait:

[Severity: High]
This is a pre-existing issue, but does this error path cause a double-free
of mhp->dereg_skb?

In c4iw_get_dma_mr(), if finish_mem_reg() fails, the error handling path calls
dereg_mem() with the pre-allocated mhp->dereg_skb. dereg_mem() passes this skb
to the network stack, where functions like c4iw_ofld_send() assume ownership
and unconditionally consume and free the skb.

When dereg_mem() returns, the error path unconditionally falls through to the
err_free_skb label and executes kfree_skb(mhp->dereg_skb) on the already-freed
pointer.

[ ... ]

> int c4iw_dereg_mr(struct ib_mr *ib_mr, struct ib_udata *udata)
> {
[ ... ]
> 	dereg_mem(&rhp->rdev, mhp->attr.stag, mhp->attr.pbl_size,
> 		  mhp->attr.pbl_addr, mhp->dereg_skb, mhp->wr_waitp);
> 	if (mhp->attr.pbl_size)
> 		c4iw_pblpool_free(&mhp->rhp->rdev, mhp->attr.pbl_addr,
> 				  mhp->attr.pbl_size << 3);
> 	if (mhp->kva)
> 		kfree((void *) (unsigned long) mhp->kva);
> 	ib_umem_release(mhp->umem);
> 	pr_debug("mmid 0x%x ptr %p\n", mmid, mhp);
> 	c4iw_put_wr_wait(mhp->wr_waitp);
> 	kfree(mhp);
> 	return 0;
> }

[Severity: Medium]
This is a pre-existing issue, but does this function leak mhp->dereg_skb if
the device is in a fatal error state?

c4iw_dereg_mr() passes mhp->dereg_skb to dereg_mem() to deregister the memory
region, assuming the network stack will consume and free the skb.

However, if the device is in a fatal error state, write_tpt_entry()
immediately aborts and returns -EIO before the skb is handed to the network
layer, leaving it unconsumed:

drivers/infiniband/hw/cxgb4/mem.c:write_tpt_entry() {
	if (c4iw_fatal_error(rdev))
		return -EIO;
    ...
}

Because c4iw_dereg_mr() ignores the return value of dereg_mem() and
unconditionally frees the mhp structure, does it permanently leak the
unconsumed skb?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724022851.466017-1-gouhao@uniontech.com?part=6

Re: [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations

[Potnuri Bharat Teja]

On Friday, July 07/24/26, 2026 at 10:28:51 +0800, Gou Hao wrote:
> alloc_skb() with the __GFP_NOFAIL flag will never return NULL,
> so the subsequent NULL checks and error handling are unreachable
> dead code. Remove them.
> 
> Signed-off-by: Gou Hao <gouhao@uniontech.com>
> ---
>  drivers/infiniband/hw/cxgb4/mem.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/cxgb4/mem.c b/drivers/infiniband/hw/cxgb4/mem.c
> index cd1b010141984..08631de17c0a3 100644
> --- a/drivers/infiniband/hw/cxgb4/mem.c
> +++ b/drivers/infiniband/hw/cxgb4/mem.c
> @@ -74,11 +74,8 @@ static int _c4iw_write_mem_dma_aligned(struct c4iw_rdev *rdev, u32 addr,
>  		c4iw_init_wr_wait(wr_waitp);
>  	wr_len = roundup(sizeof(*req) + sizeof(*sgl), 16);
>  
> -	if (!skb) {
> +	if (!skb)
>  		skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
> -		if (!skb)
> -			return -ENOMEM;
> -	}
>  	set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
>  
>  	req = __skb_put_zero(skb, wr_len);
> @@ -134,11 +131,8 @@ static int _c4iw_write_mem_inline(struct c4iw_rdev *rdev, u32 addr, u32 len,
>  					 roundup(copy_len, T4_ULPTX_MIN_IO),
>  				 16);
>  
> -		if (!skb) {
> +		if (!skb)
>  			skb = alloc_skb(wr_len, GFP_KERNEL | __GFP_NOFAIL);
> -			if (!skb)
> -				return -ENOMEM;
> -		}

Reviewed-by: Potnuri Bharat Teja <bharat@chelsio.com>

>  		set_wr_txq(skb, CPL_PRIORITY_CONTROL, 0);
>  
>  		req = __skb_put_zero(skb, wr_len);
> -- 
> 2.20.1
>