[PATCH net] dibs: fix double free of dmb_clientid_arr

[PATCH net] 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().

Note: commit cc21191b584c ("dibs: Move data path to dibs layer") moved
the code to its current location; the race was introduced earlier by
commit c3a910f2380f ("net/smc: implement DMB-merged operations of
loopback-ism").

Fixes: c3a910f2380f ("net/smc: implement DMB-merged operations of loopback-ism")
Cc: stable@vger.kernel.org
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
 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: 98379cf6109c0adb5879f86ba3efd4afe389a64a
prerequisite-patch-id: da145e40ff1ce22e9701dd49676606f612c03872
-- 
2.52.0

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

[sashiko-bot]

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

Pre-existing issues:
- [High] The hardware interrupt handler `ism_handle_irq()` can execute out-of-order with respect to initialization and teardown of critical resources, leading to crashes (UAF, NULL deref, and uninitialized spinlock accesses).
- [High] `dibs_unregister_client()` leaves the subsystem in an inconsistent partial state if it aborts midway through its device iteration.
- [High] `dibs_lo_attach_dmb` drops the hash table read lock before incrementing the object's reference count, leading to a Use-After-Free.
--

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

dibs: fix double free of dmb_clientid_arr

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

Cc: stable@vger.kernel.org

> diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
> index 0f2e093111526..aeb8787ea9816 100644
> --- a/drivers/dibs/dibs_loopback.c
> +++ b/drivers/dibs/dibs_loopback.c
> @@ -334,7 +334,6 @@ static int dibs_lo_dev_probe(void)
>  	return 0;
>  
>  err_reg:
> -	kfree(dibs->dmb_clientid_arr);

[Severity: High]
This isn't a bug introduced by this patch, but does dibs_lo_attach_dmb() have
a use-after-free if the reference count is incremented after dropping the hash
table lock?

drivers/dibs/dibs_loopback.c:dibs_lo_attach_dmb() {
...
	read_unlock_bh(&ldev->dmb_ht_lock);

	if (!refcount_inc_not_zero(&dmb_node->refcnt))
...
}

If a concurrent thread executing dibs_lo_unregister_dmb() acquires the lock
and drops the refcount to zero, could it free the node before the first thread
resumes, causing refcount_inc_not_zero() to operate on freed memory?

[Severity: High]
This is a pre-existing issue, but is there an initialization race with the
hardware interrupt handler in ism_drv.c?

drivers/s390/net/ism_drv.c:ism_probe() {
...
	ret = dibs_dev_add(dibs);
...
}

If the hardware interrupt is registered before dibs_dev_add() initializes
resources, and a spurious interrupt fires, could ism_handle_irq() access an
uninitialized spinlock or dereference a NULL pointer?

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

Similarly, during teardown:

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

Could freeing resources like dibs->dmb_clientid_arr via dibs_dev_del() before
the active IRQ is unregistered create a use-after-free window?

[Severity: High]
This is also a pre-existing issue, but does dibs_unregister_client() leave the
subsystem in an inconsistent state if it aborts early?

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

If the loop successfully processes some devices but then encounters a device
with active DMBs, it returns -EBUSY without rolling back the teardown on the
previously processed devices. If the caller ignores this return value and
proceeds to unload the module, could the detached devices later trigger an
event and dereference freed ops function pointers?

>  	/* pairs with dibs_dev_alloc() */
>  	put_device(&dibs->dev);
>  	kfree(ldev);

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

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

[Alexandra Winter]

On 31.07.26 09:56, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The hardware interrupt handler `ism_handle_irq()` can execute out-of-order with respect to initialization and teardown of critical resources, leading to crashes (UAF, NULL deref, and uninitialized spinlock accesses).
> - [High] `dibs_unregister_client()` leaves the subsystem in an inconsistent partial state if it aborts midway through its device iteration.
> - [High] `dibs_lo_attach_dmb` drops the hash table read lock before incrementing the object's reference count, leading to a Use-After-Free.
> --
> 
> --- Patch [1]: [PATCH net] dibs: fix double free of dmb_clientid_arr ---
> commit 45565b8a1cec1239be540b3b54fad60822ae8b57
> Author: Hidayath Khan <hidayath@linux.ibm.com>
> 
> dibs: fix double free of dmb_clientid_arr
> 
> 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().
> 
> Cc: stable@vger.kernel.org
> 
>> diff --git a/drivers/dibs/dibs_loopback.c b/drivers/dibs/dibs_loopback.c
>> index 0f2e093111526..aeb8787ea9816 100644
>> --- a/drivers/dibs/dibs_loopback.c
>> +++ b/drivers/dibs/dibs_loopback.c
>> @@ -334,7 +334,6 @@ static int dibs_lo_dev_probe(void)
>>  	return 0;
>>  
>>  err_reg:
>> -	kfree(dibs->dmb_clientid_arr);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does dibs_lo_attach_dmb() have
> a use-after-free if the reference count is incremented after dropping the hash
> table lock?
> 
> drivers/dibs/dibs_loopback.c:dibs_lo_attach_dmb() {
> ...
> 	read_unlock_bh(&ldev->dmb_ht_lock);
> 
> 	if (!refcount_inc_not_zero(&dmb_node->refcnt))
> ...
> }

Fixed by
a10ea943356b ("dibs: fix use-after-free of dmb_node in loopback attach/detach/unregister")
that got accepted to net yesterday.


> 
> If a concurrent thread executing dibs_lo_unregister_dmb() acquires the lock
> and drops the refcount to zero, could it free the node before the first thread
> resumes, causing refcount_inc_not_zero() to operate on freed memory?
> 
> [Severity: High]
> This is a pre-existing issue, but is there an initialization race with the
> hardware interrupt handler in ism_drv.c?
> 
> drivers/s390/net/ism_drv.c:ism_probe() {
> ...
> 	ret = dibs_dev_add(dibs);
> ...
> }
> 
> If the hardware interrupt is registered before dibs_dev_add() initializes
> resources, and a spurious interrupt fires, could ism_handle_irq() access an
> uninitialized spinlock or dereference a NULL pointer?
> 

Hidayath has already sent a fix for the lock initialization:
https://lore.kernel.org/netdev/20260730124227.167829-1-hidayath@linux.ibm.com/



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


No client can have registered a dmb at that point, so
no dmb interrupt can occur, so this NULL pointer dereference cannot happen irl.
I plan to restructure with a future cleanup patch.


> 
> Similarly, during teardown:
> 
> drivers/s390/net/ism_drv.c:ism_remove() {
> ...
> 	dibs_dev_del(dibs);
> 	ism_dev_exit(ism);
> ...
> }
> 
> Could freeing resources like dibs->dmb_clientid_arr via dibs_dev_del() before
> the active IRQ is unregistered create a use-after-free window?
> 


Yes, we will send a fix.


> [Severity: High]
> This is also a pre-existing issue, but does dibs_unregister_client() leave the
> subsystem in an inconsistent state if it aborts early?
> 
> drivers/dibs/dibs_main.c:dibs_unregister_client() {
> ...
> 				rc = -EBUSY;
> 				goto err_reg_dmb;
> ...
> }
> 
> If the loop successfully processes some devices but then encounters a device
> with active DMBs, it returns -EBUSY without rolling back the teardown on the
> previously processed devices. If the caller ignores this return value and
> proceeds to unload the module, could the detached devices later trigger an
> event and dereference freed ops function pointers?
> 
>>  	/* pairs with dibs_dev_alloc() */
>>  	put_device(&dibs->dev);
>>  	kfree(ldev);
> 


Does not happen with smc today.
But the guard is not clean, it should do a better and safer cleanup,
should such a situation arise with a future client.
I will work on an improvement.

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

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260730075624.114778-1-hidayath@linux.ibm.com?part=1
---
--- Patch [1]: [PATCH net] dibs: fix double free of dmb_clientid_arr ---
commit d9da866ee3cff93c660daf037f85b099a2326866
Author: Hidayath Khan <hidayath@linux.ibm.com>

dibs: fix double free of dmb_clientid_arr

This drops the kfree(dibs->dmb_clientid_arr) from the err_reg path of
dibs_lo_dev_probe(), since dibs_dev_add() already frees that buffer on
both of its own failure paths without clearing the pointer.

Two questions below, one about the commit message tags and one about the
lifetime of the same buffer on the removal path.

> dibs: fix double free of dmb_clientid_arr
>
> 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().
>
> Note: commit cc21191b584c ("dibs: Move data path to dibs layer") moved
> the code to its current location; the race was introduced earlier by
> commit c3a910f2380f ("net/smc: implement DMB-merged operations of
> loopback-ism").
>
> Fixes: c3a910f2380f ("net/smc: implement DMB-merged operations of loopback-ism")
> Cc: stable@vger.kernel.org

[Severity: Medium]
[Finding: main-1-0]
[Sources: opus-5]
Is c3a910f2380f the right commit to blame here?

That commit only touches net/smc/smc_loopback.c and net/smc/smc_loopback.h,
adding DMB refcounting plus attach/detach/nocopy support. It creates no
client-id array, and it adds no kfree() on any probe error path. At
c3a910f2380f^ the only matching array in the tree is ISM's private
sba_client_arr in drivers/s390/net/ism_drv.c and include/linux/ism.h.

Blame instead points both halves of the double free at cc21191b584c
("dibs: Move data path to dibs layer"): the allocation and the free in
dibs_dev_add(),

	dibs->dmb_clientid_arr = kzalloc(max_dmbs, GFP_KERNEL);
	...
	free_client_arr:
		kfree(dibs->dmb_clientid_arr);

and the duplicate kfree() in dibs_lo_dev_probe()'s err_reg path that this
patch removes. Would it be more accurate to say cc21191b584c introduced the
problem, rather than that it "moved the code to its current location; the
race was introduced earlier"?

With Cc: stable@vger.kernel.org present, a Fixes tag naming a v6.10-era
net/smc commit points backports at trees where neither drivers/dibs/ nor
dmb_clientid_arr exists, so the patch cannot apply there.

Also, the note calls this a "race", but the double free happens
unconditionally on a deterministic error path when device_add() or
sysfs_create_group() fails. Would "double free" be the better wording?

> 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]
[Finding: main-5-4]
[Sources: opus-5]
This isn't a bug introduced by this patch, but while looking at the
lifetime of dmb_clientid_arr: does the removal path have the same missing
NULL assignment, and can the ISM interrupt handler read the buffer after it
is freed?

dibs_dev_del() in drivers/dibs/dibs_main.c ends with the free outside
dibs->lock and leaves the pointer set:

	device_del(&dibs->dev);
	kfree(dibs->dmb_clientid_arr);

The spin_lock_irqsave(&dibs->lock) section earlier in that function only
clears dibs->subs[], and is released well before the kfree().

ism_remove() in drivers/s390/net/ism_drv.c then runs:

	dibs_dev_del(dibs);
	ism_dev_exit(ism);

and ism_dev_exit() is the only place that calls unregister_sba() and
free_irq(pci_irq_vector(pdev, 0), ism). So the SBA stays registered and the
MSI handler stays armed across the kfree(). ism_handle_irq() does, for every
set DMB bit while holding dibs->lock:

	client_id = dibs->dmb_clientid_arr[bit];
	if (unlikely(client_id == NO_DIBS_CLIENT ||
		     !dibs->subs[client_id]))
		continue;
	dibs->subs[client_id]->ops->handle_irq(dibs, ...);

Since the free does not take dibs->lock, can the handler be mid-loop while
the removal path frees the array underneath it?

There is a second effect if the byte read back is garbage: only
NO_DIBS_CLIENT is rejected, while subs[] has MAX_DIBS_CLIENTS == 8 entries
and is the last member of struct dibs_dev in include/linux/dibs.h. Can a
client_id in 8..254 read past the end of the kmalloc'ed struct dibs_dev and,
if that slot is non-NULL, produce an indirect call through a bogus struct
dibs_client pointer?

For the window to be reachable a DMB bit must still be settable by the peer.
ism_unregister_dmb() writes NO_DIBS_CLIENT and then issues the hardware
unregister command, but:

	ret = ism_cmd(ism, &cmd);
	if (ret && ret != ISM_ERROR)
		goto out;

leaves the DMB registered in the adapter when the command fails, so a remote
peer can keep signalling it.

Would it make sense to free and NULL dmb_clientid_arr only after the owning
driver has quiesced its interrupt and data path, do it under dibs->lock, and
bound-check client_id against MAX_DIBS_CLIENTS in ism_handle_irq()? The
missing NULL assignment in dibs_dev_add() is also what turned the removed
kfree() into a double free.

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

[Alexandra Winter]

On 04.08.26 18:52, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260730075624.114778-1-hidayath@linux.ibm.com?part=1
> ---
> --- Patch [1]: [PATCH net] dibs: fix double free of dmb_clientid_arr ---
> commit d9da866ee3cff93c660daf037f85b099a2326866
> Author: Hidayath Khan <hidayath@linux.ibm.com>
> 
> dibs: fix double free of dmb_clientid_arr
> 
> This drops the kfree(dibs->dmb_clientid_arr) from the err_reg path of
> dibs_lo_dev_probe(), since dibs_dev_add() already frees that buffer on
> both of its own failure paths without clearing the pointer.
> 
> Two questions below, one about the commit message tags and one about the
> lifetime of the same buffer on the removal path.
> 
>> dibs: fix double free of dmb_clientid_arr
>>
>> 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().
>>
>> Note: commit cc21191b584c ("dibs: Move data path to dibs layer") moved
>> the code to its current location; the race was introduced earlier by
>> commit c3a910f2380f ("net/smc: implement DMB-merged operations of
>> loopback-ism").
>>
>> Fixes: c3a910f2380f ("net/smc: implement DMB-merged operations of loopback-ism")
>> Cc: stable@vger.kernel.org
> 
> [Severity: Medium]
> [Finding: main-1-0]
> [Sources: opus-5]
> Is c3a910f2380f the right commit to blame here?
> 
> That commit only touches net/smc/smc_loopback.c and net/smc/smc_loopback.h,
> adding DMB refcounting plus attach/detach/nocopy support. It creates no
> client-id array, and it adds no kfree() on any probe error path. At
> c3a910f2380f^ the only matching array in the tree is ISM's private
> sba_client_arr in drivers/s390/net/ism_drv.c and include/linux/ism.h.
> 
> Blame instead points both halves of the double free at cc21191b584c
> ("dibs: Move data path to dibs layer"): the allocation and the free in
> dibs_dev_add(),
> 
> 	dibs->dmb_clientid_arr = kzalloc(max_dmbs, GFP_KERNEL);
> 	...
> 	free_client_arr:
> 		kfree(dibs->dmb_clientid_arr);
> 
> and the duplicate kfree() in dibs_lo_dev_probe()'s err_reg path that this
> patch removes. Would it be more accurate to say cc21191b584c introduced the
> problem, rather than that it "moved the code to its current location; the
> race was introduced earlier"?
> 

Hidayath,
I think, AI is right here, the original smc_loopback.c did not have such a construct.
Only cc21191b584c is to blame.
(Seems you errounously copied this from your other patch
a10ea943356b ("dibs: fix use-after-free of dmb_node in loopback attach/detach/unregister")
Please send a v2



> With Cc: stable@vger.kernel.org present, a Fixes tag naming a v6.10-era
> net/smc commit points backports at trees where neither drivers/dibs/ nor
> dmb_clientid_arr exists, so the patch cannot apply there.
> 
> Also, the note calls this a "race", but the double free happens
> unconditionally on a deterministic error path when device_add() or
> sysfs_create_group() fails. Would "double free" be the better wording?
> 


Correct (but moot, if this paragraph is removed)


>> 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]
> [Finding: main-5-4]
> [Sources: opus-5]
> This isn't a bug introduced by this patch, but while looking at the
> lifetime of dmb_clientid_arr: does the removal path have the same missing
> NULL assignment, and can the ISM interrupt handler read the buffer after it
> is freed?
> 


[PATCH net] net/dibs: Fix UAF of dmb_clientid_arr after dibs_dev_del()
is currently discussed on the netdev mailing list to fix this.

[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

[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

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