* [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free
@ 2026-07-24 2:28 Gou Hao
2026-07-24 2:28 ` [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation Gou Hao
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 4:55 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe() Gou Hao
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe()
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
2026-07-24 2:28 ` [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 4:55 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis Gou Hao
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
2026-07-24 2:28 ` [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation Gou Hao
2026-07-24 2:28 ` [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe() Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 2:44 ` sashiko-bot
2026-07-24 4:58 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 4/6] drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation Gou Hao
` (2 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/6] drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
` (2 preceding siblings ...)
2026-07-24 2:28 ` [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 2:28 ` [PATCH v2 5/6] lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations Gou Hao
2026-07-24 2:28 ` [PATCH v2 6/6] RDMA/cxgb4: " Gou Hao
5 siblings, 0 replies; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 5/6] lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
` (3 preceding siblings ...)
2026-07-24 2:28 ` [PATCH v2 4/6] drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 2:28 ` [PATCH v2 6/6] RDMA/cxgb4: " Gou Hao
5 siblings, 0 replies; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
` (4 preceding siblings ...)
2026-07-24 2:28 ` [PATCH v2 5/6] lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations Gou Hao
@ 2026-07-24 2:28 ` Gou Hao
2026-07-24 2:53 ` sashiko-bot
2026-07-24 7:34 ` Potnuri Bharat Teja
5 siblings, 2 replies; 13+ messages in thread
From: Gou Hao @ 2026-07-24 2:28 UTC (permalink / raw)
To: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, gouhao,
namcao, ynorov, sshegde, nilay, clg
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis
2026-07-24 2:28 ` [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis Gou Hao
@ 2026-07-24 2:44 ` sashiko-bot
2026-07-24 4:58 ` Cédric Le Goater
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-24 2:44 UTC (permalink / raw)
To: Gou Hao; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations
2026-07-24 2:28 ` [PATCH v2 6/6] RDMA/cxgb4: " Gou Hao
@ 2026-07-24 2:53 ` sashiko-bot
2026-07-24 7:34 ` Potnuri Bharat Teja
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-07-24 2:53 UTC (permalink / raw)
To: Gou Hao; +Cc: dri-devel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation
2026-07-24 2:28 ` [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation Gou Hao
@ 2026-07-24 4:55 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2026-07-24 4:55 UTC (permalink / raw)
To: Gou Hao, maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, namcao,
ynorov, sshegde, nilay
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe()
2026-07-24 2:28 ` [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe() Gou Hao
@ 2026-07-24 4:55 ` Cédric Le Goater
0 siblings, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2026-07-24 4:55 UTC (permalink / raw)
To: Gou Hao, maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, namcao,
ynorov, sshegde, nilay
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis
2026-07-24 2:28 ` [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis Gou Hao
2026-07-24 2:44 ` sashiko-bot
@ 2026-07-24 4:58 ` Cédric Le Goater
1 sibling, 0 replies; 13+ messages in thread
From: Cédric Le Goater @ 2026-07-24 4:58 UTC (permalink / raw)
To: Gou Hao, maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, bharat, jgg, leon, akpm, namcao,
ynorov, sshegde, nilay
Cc: linuxppc-dev, linux-kernel, dri-devel, linux-rdma, linux-mm,
gouhaojake, kernel
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 6/6] RDMA/cxgb4: remove dead NULL checks after GFP_NOFAIL allocations
2026-07-24 2:28 ` [PATCH v2 6/6] RDMA/cxgb4: " Gou Hao
2026-07-24 2:53 ` sashiko-bot
@ 2026-07-24 7:34 ` Potnuri Bharat Teja
1 sibling, 0 replies; 13+ messages in thread
From: Potnuri Bharat Teja @ 2026-07-24 7:34 UTC (permalink / raw)
To: Gou Hao
Cc: maddy, mpe, npiggin, chleroy, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, jgg, leon, akpm, namcao, ynorov,
sshegde, nilay, clg, linuxppc-dev, linux-kernel, dri-devel,
linux-rdma, linux-mm, gouhaojake, kernel
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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-07-24 7:41 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 2:28 [PATCH v2 0/6] remove dead NULL checks after GFP_NOFAIL allocations and fix xive use-after-free Gou Hao
2026-07-24 2:28 ` [PATCH v2 1/6] powerpc/xive: remove dead NULL check after GFP_NOFAIL allocation Gou Hao
2026-07-24 4:55 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 2/6] powerpc/xive: add error return value to xive_smp_probe() Gou Hao
2026-07-24 4:55 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 3/6] powerpc/xive: fix use-after-free of xive_ipis Gou Hao
2026-07-24 2:44 ` sashiko-bot
2026-07-24 4:58 ` Cédric Le Goater
2026-07-24 2:28 ` [PATCH v2 4/6] drm: remove dead WARN_ON NULL check after GFP_NOFAIL allocation Gou Hao
2026-07-24 2:28 ` [PATCH v2 5/6] lib/test_hmm: remove dead NULL checks after GFP_NOFAIL allocations Gou Hao
2026-07-24 2:28 ` [PATCH v2 6/6] RDMA/cxgb4: " Gou Hao
2026-07-24 2:53 ` sashiko-bot
2026-07-24 7:34 ` Potnuri Bharat Teja
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.