[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

[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

[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

[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 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 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 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.

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.

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.

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
> 

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

[Andrew Morton]

On Fri, 24 Jul 2026 10:28:45 +0800 Gou Hao <gouhao@uniontech.com> wrote:

> 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.

Sigh.

Back when Linux ran on an abacus, I added __GFP_NOFAIL because four or
five sites (file systems) were infinitely looping on alloc_pages until
it succeeded.  I figured we should move this operation into the page
allocator so we could easily find those sites and fix them to Not Do
That.

I made it very clear (changelog and code comments) that no new code
should use __GFP_NOFAIL.  How did that work out?

hp2:/usr/src/linux-7.2-rc4> grep -r __GFP_NOFAIL | wc -l
241

So much lameness!   My immediate reaction to your patchset is "No.  Fix
the damn code instead".  But I've resoundingly lost that fight.


Thanks, I'll scoop up the entire series.  If individual patches pop up
in linux-next then I'll drop the mm.git copies.



AI review has a shower of possible pre-existing issues.  I'm seeing
use-after-free, double-free, leak, system hang, NULL pointer
dereference, usual stuff :(

They're in powerpc and cxgb4, if maintainers are interested.

	https://sashiko.dev/#/patchset/20260724022851.466017-1-gouhao@uniontech.com

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

[Leon Romanovsky]

On Fri, 24 Jul 2026 10:28:45 +0800, Gou Hao wrote:
> 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().
> 
> [...]

Applied, thanks!

[6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations
      https://git.kernel.org/rdma/rdma/c/a273210687f255

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>

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

[Leon Romanovsky]

On Sat, Jul 25, 2026 at 08:26:32PM -0700, Andrew Morton wrote:
> On Fri, 24 Jul 2026 10:28:45 +0800 Gou Hao <gouhao@uniontech.com> wrote:
> 
> > 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.
> 
> Sigh.
> 
> Back when Linux ran on an abacus, I added __GFP_NOFAIL because four or
> five sites (file systems) were infinitely looping on alloc_pages until
> it succeeded.  I figured we should move this operation into the page
> allocator so we could easily find those sites and fix them to Not Do
> That.
> 
> I made it very clear (changelog and code comments) that no new code
> should use __GFP_NOFAIL.  How did that work out?
> 
> hp2:/usr/src/linux-7.2-rc4> grep -r __GFP_NOFAIL | wc -l
> 241
> 
> So much lameness!   My immediate reaction to your patchset is "No.  Fix
> the damn code instead".  But I've resoundingly lost that fight.
> 
> 
> Thanks, I'll scoop up the entire series.  If individual patches pop up
> in linux-next then I'll drop the mm.git copies.
> 
> 
> 
> AI review has a shower of possible pre-existing issues.  I'm seeing
> use-after-free, double-free, leak, system hang, NULL pointer
> dereference, usual stuff :(
> 
> They're in powerpc and cxgb4, if maintainers are interested.
> 
> 	https://sashiko.dev/#/patchset/20260724022851.466017-1-gouhao@uniontech.com

Regarding cxgb4, yes, I'm looking into it.

Thanks