[PATCH net v2] dibs: fix double free of dmb_clientid_arr

[PATCH net v2] dibs: fix double free of dmb_clientid_arr

[Hidayath Khan]

dibs_dev_add() frees dibs->dmb_clientid_arr on both of its failure
paths, but does not clear the pointer. dibs_lo_dev_probe() then frees
the same pointer again in its err_reg path, so a failing device_add()
or sysfs_create_group() results in a double free.

Drop the kfree() from dibs_lo_dev_probe().

Fixes: cc21191b584c ("dibs: Move data path to dibs layer")
Cc: stable@vger.kernel.org
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
v2:
- Fix Fixes: tag (was c3a910f2380f, should be cc21191b584c)
- Remove incorrect Note paragraph copied from a different patch

 drivers/dibs/dibs_loopback.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
index fd5caf1e19a8..649e4e375be3 100644
--- a/drivers/dibs/dibs_loopback.c
+++ b/drivers/dibs/dibs_loopback.c
@@ -335,7 +335,6 @@ static int dibs_lo_dev_probe(void)
 	return 0;
 
 err_reg:
-	kfree(dibs->dmb_clientid_arr);
 	/* pairs with dibs_dev_alloc() */
 	put_device(&dibs->dev);
 	kfree(ldev);

base-commit: 92a09f2af870e91efb9471b109db15c2030f90f2
-- 
2.52.0

[PATCH net v2] dibs: fix double free of dmb_clientid_arr

[Hidayath Khan]

dibs_dev_add() frees dibs->dmb_clientid_arr on both of its failure
paths, but does not clear the pointer. dibs_lo_dev_probe() then frees
the same pointer again in its err_reg path, so a failing device_add()
or sysfs_create_group() results in a double free.

Drop the kfree() from dibs_lo_dev_probe().

Fixes: cc21191b584c ("dibs: Move data path to dibs layer")
Cc: stable@vger.kernel.org
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
v2:
- Fix Fixes: tag (was c3a910f2380f, should be cc21191b584c)
- Remove incorrect Note paragraph copied from a different patch

 drivers/dibs/dibs_loopback.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
index fd5caf1e19a8..649e4e375be3 100644
--- a/drivers/dibs/dibs_loopback.c
+++ b/drivers/dibs/dibs_loopback.c
@@ -335,7 +335,6 @@ static int dibs_lo_dev_probe(void)
 	return 0;
 
 err_reg:
-	kfree(dibs->dmb_clientid_arr);
 	/* pairs with dibs_dev_alloc() */
 	put_device(&dibs->dev);
 	kfree(ldev);

base-commit: 92a09f2af870e91efb9471b109db15c2030f90f2
-- 
2.52.0

Re: [PATCH net v2] dibs: fix double free of dmb_clientid_arr

[Dust Li]

On 2026-08-06 14:04:47, Hidayath Khan wrote:
>dibs_dev_add() frees dibs->dmb_clientid_arr on both of its failure
>paths, but does not clear the pointer. dibs_lo_dev_probe() then frees
>the same pointer again in its err_reg path, so a failing device_add()
>or sysfs_create_group() results in a double free.
>
>Drop the kfree() from dibs_lo_dev_probe().
>
>Fixes: cc21191b584c ("dibs: Move data path to dibs layer")
>Cc: stable@vger.kernel.org
>Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
>Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>

Reviewed-by: Dust Li <dust.li@linux.alibaba.com>

Best regards,
Dust

Re: [PATCH net v2] dibs: fix double free of dmb_clientid_arr

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Hardware interrupt is registered before required data structures and locks are initialized.
- [Critical] Asymmetric teardown order creates a Use-After-Free window in `ism_remove()` and probe error paths.
- [High] Missing `class_unregister(&dibs_class)` on the error path in `dibs_init()`.
- [High] Incomplete rollback in `dibs_unregister_client()` leaves client in a partially unregistered state.
--

--- Patch [1]: [PATCH net v2] dibs: fix double free of dmb_clientid_arr ---
commit 1001308ced15383d91624271c00a3bbcf60208a4
Author: Hidayath Khan <hidayath@linux.ibm.com>

dibs: fix double free of dmb_clientid_arr

This commit fixes a double free of dibs->dmb_clientid_arr by dropping
the kfree() in the err_reg path of dibs_lo_dev_probe(). The pointer is
already freed by dibs_dev_add() on failure paths.

> diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
> index fd5caf1e19a8f..649e4e375be33 100644
> --- a/drivers/dibs/dibs_loopback.c
> +++ b/drivers/dibs/dibs_loopback.c
> @@ -335,7 +335,6 @@ static int dibs_lo_dev_probe(void)
>  	return 0;
>  
>  err_reg:
> -	kfree(dibs->dmb_clientid_arr);
>  	/* pairs with dibs_dev_alloc() */
>  	put_device(&dibs->dev);
>  	kfree(ldev);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a missing
class_unregister() on the error path in dibs_init(). 

If dibs_loopback_init() fails, it returns an error without cleaning up the
registered class:

drivers/dibs/dibs_main.c:dibs_init() {
    ...
	rc = dibs_loopback_init();
	if (rc)
		pr_err("%s fails with %d\n", __func__, rc);

	return rc;
}

Can this lead to a use-after-free? Any subsequent interaction with
/sys/class/dibs or reloading the module might dereference the freed class
struct.

[Severity: High]
This is a pre-existing issue, but incomplete rollback in
dibs_unregister_client() seems to leave the client in a partially
unregistered state:

drivers/dibs/dibs_main.c:dibs_unregister_client() {
    ...
				rc = -EBUSY;
				goto err_reg_dmb;
			}
		}
    ...
}

If it encounters a device with active DMBs, it returns -EBUSY immediately
without restoring the state of previously unregistered devices. Does this
permanently corrupt the client state where it misses events for those
earlier devices while believing it is still fully registered?

[Severity: Critical]
This isn't a bug introduced by this patch, but the hardware interrupt
appears to be registered before the required data structures and locks are
initialized:

drivers/s390/net/ism_drv.c:ism_probe() {
    ...
	ret = ism_dev_init(ism);
	if (ret)
		goto err_dibs;
    ...
}

ism_dev_init() registers the hardware IRQ before dibs_dev_add() initializes
dibs->lock and dibs->dmb_clientid_arr. If a spurious or pending interrupt
fires during this small window:

drivers/s390/net/ism_drv.c:ism_handle_irq() {
    ...
	spin_lock(&dibs->lock);
...
		client_id = dibs->dmb_clientid_arr[bit];
    ...
}

Will this cause a panic due to attempting to acquire an uninitialized
spinlock and dereferencing the NULL dmb_clientid_arr pointer?

[Severity: Critical]
This is a pre-existing issue, but there seems to be an asymmetric teardown
order creating a use-after-free window in ism_remove():

drivers/s390/net/ism_drv.c:ism_remove() {
    ...
	dibs_dev_del(dibs);
	ism_dev_exit(ism);
    ...
}

dibs_dev_del() frees dibs->dmb_clientid_arr:

drivers/dibs/dibs_main.c:dibs_dev_del() {
    ...
	kfree(dibs->dmb_clientid_arr);
    ...
}

However, the interrupt is only disabled later in ism_dev_exit(). 

If an interrupt fires in this window, ism_handle_irq() will read from the
freed array. Could garbage data interpreted as client_id cause an
out-of-bounds access when indexing dibs->subs[client_id]?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806120002.596137-1-hidayath@linux.ibm.com?part=1

Re: [PATCH net v2] dibs: fix double free of dmb_clientid_arr

[sashiko-bot]

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

Pre-existing issues:
- [High] dibs_unregister_client() fails with partial unregistration if an active DMB is found on a device later in the list, leaving the client in a corrupted state.
- [High] Initialization failure in dibs_init() leaks dibs_class and causes Use-After-Free.
- [High] Early registration of ism_handle_irq() in ism_probe() causes potential NULL Pointer Dereference and Use-After-Free.
- [High] `dibs_lo_move_data()` acquires `dibs->lock` with plain `spin_lock()`, missing IRQ masking.
- [High] In the error path of `ism_alloc_dmb()`, a folio allocated page is illegally freed using `kfree()`.
--

--- Patch [1]: [PATCH net v2] dibs: fix double free of dmb_clientid_arr ---
commit c4dab923b3875e2fdb8522fd524541f2cd53cf25
Author: Hidayath Khan <hidayath@linux.ibm.com>
Subject: dibs: fix double free of dmb_clientid_arr

This commit removes the kfree() call in dibs_lo_dev_probe() to prevent a
double free of dibs->dmb_clientid_arr. The pointer is already freed on both
failure paths of dibs_dev_add(), which is called just before the error path.

> diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
> index fd5caf1e19a8f..649e4e375be33 100644
> --- a/drivers/dibs/dibs_loopback.c
> +++ b/drivers/dibs/dibs_loopback.c
> @@ -335,7 +335,6 @@ static int dibs_lo_dev_probe(void)
>  	return 0;
>  
>  err_reg:
> -	kfree(dibs->dmb_clientid_arr);
>  	/* pairs with dibs_dev_alloc() */
>  	put_device(&dibs->dev);
>  	kfree(ldev);

While reviewing this area, I noticed a few pre-existing issues that were not
introduced by this patch.

drivers/dibs/dibs_main.c:dibs_unregister_client()
> 		for (int i = 0; i < max_dmbs; ++i) {
> 			if (dibs->dmb_clientid_arr[i] == client->id) {
> 				WARN(1, "%s: attempt to unregister '%s' with registered dmb(s)\n",
> 				     __func__, client->name);
> 				rc = -EBUSY;
> 				goto err_reg_dmb;
> 			}
> 		}
> 		/* Stop forwarding IRQs and events */
> 		dibs->subs[client->id] = NULL;
> 		spin_unlock_irqrestore(&dibs->lock, flags);
> 		clients[client->id]->ops->del_dev(dibs);

[Severity: High]
This isn't a bug introduced by this patch, but if dibs_unregister_client()
finds an active DMB on a device later in the list, it aborts and returns
-EBUSY. Does this leave the client in a corrupted state since the earlier
devices in the list already had their forwarding stopped and del_dev() called?
A subsequent retry could result in a double-free or use-after-free when
del_dev() is called again for the already processed devices.

drivers/dibs/dibs_main.c:dibs_init()
> static int __init dibs_init(void)
> {
> 	int rc;
> 
> 	rc = class_register(&dibs_class);
> 	if (rc)
> 		return rc;
> 
> 	rc = dibs_loopback_init();
> 	if (rc)
> 		pr_err("%s fails with %d\n", __func__, rc);
> 
> 	return rc;
> }

[Severity: High]
This is a pre-existing issue, but does an initialization failure here leak the
dibs_class? If dibs_loopback_init() fails, the module load is aborted and
memory is freed, but class_register() isn't undone. Could this result in a
use-after-free if sysfs accesses the freed module memory?

drivers/s390/net/ism_drv.c:ism_probe()
> 	/* enable ism device, but any interrupts and events will be ignored
> 	 * before dibs_dev_add() adds it to any clients.
> 	 */
> 	ret = ism_dev_init(ism);
> 	if (ret)
> 		goto err_dibs;
> ...
> 	ret = dibs_dev_add(dibs);

drivers/s390/net/ism_drv.c:ism_handle_irq()
> 	spin_lock(&dibs->lock);
> 	ism->sba->s = 0;
> 	barrier();
> ...
> 		client_id = dibs->dmb_clientid_arr[bit];

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for an early
interrupt to trigger a NULL pointer dereference or use-after-free?
ism_dev_init() enables the hardware interrupts before dibs_dev_add()
initializes dibs->lock and allocates dibs->dmb_clientid_arr. If an interrupt
fires early, could ism_handle_irq() attempt to acquire an uninitialized lock
and dereference a NULL pointer?

drivers/dibs/dibs_loopback.c:dibs_lo_move_data()
> 	spin_lock(&dibs->lock);
> 	client_id = dibs->dmb_clientid_arr[sba_idx];
> 	s_mask = ror16(0x1000, idx);
> 	if (likely(client_id != NO_DIBS_CLIENT && dibs->subs[client_id]))
> 		dibs->subs[client_id]->ops->handle_irq(dibs, sba_idx, s_mask);
> 	spin_unlock(&dibs->lock);

[Severity: High]
This is a pre-existing issue, but is it safe to acquire dibs->lock using
spin_lock() here? Since the lock is also acquired in the hardirq context by
ism_handle_irq(), could acquiring it here without disabling interrupts cause a
deadlock if an interrupt arrives on the same CPU?

drivers/s390/net/ism_drv.c:ism_alloc_dmb()
> 	folio = folio_alloc(GFP_KERNEL | __GFP_NOWARN | __GFP_NOMEMALLOC |
> 			    __GFP_NORETRY, get_order(dmb->dmb_len));
> 
> 	if (!folio) {
> 		rc = -ENOMEM;
> 		goto out_bit;
> 	}
> 
> 	dmb->cpu_addr = folio_address(folio);
> 	dmb->dma_addr = dma_map_page(&ism->pdev->dev,
> 				     virt_to_page(dmb->cpu_addr), 0,
> 				     dmb->dmb_len, DMA_FROM_DEVICE);
> 	if (dma_mapping_error(&ism->pdev->dev, dmb->dma_addr)) {
> 		rc = -ENOMEM;
> 		goto out_free;
> 	}
> 
> 	return 0;
> 
> out_free:
> 	kfree(dmb->cpu_addr);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path incorrectly
free the page memory? Since dmb->cpu_addr was allocated via folio_alloc(),
shouldn't it be freed using folio_put() instead of kfree() to avoid slab
allocator corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806120447.596164-1-hidayath@linux.ibm.com?part=1

Re: [PATCH net v2] dibs: fix double free of dmb_clientid_arr

[Jakub Kicinski]

On Thu,  6 Aug 2026 14:04:47 +0200 Hidayath Khan wrote:
> dibs_dev_add() frees dibs->dmb_clientid_arr on both of its failure
> paths, but does not clear the pointer. dibs_lo_dev_probe() then frees
> the same pointer again in its err_reg path, so a failing device_add()
> or sysfs_create_group() results in a double free.
> 
> Drop the kfree() from dibs_lo_dev_probe().

This should be squashed with Alexandra's fix.