mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work()
@ 2026-09-23 17:23 Takashi Sakamoto
  2026-09-23 17:23 ` [PATCH 1/2] firewire: cdev: hold client reference for fw_iso_resource_auto lifetime Takashi Sakamoto
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2026-09-23 17:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: dingiso.kernel, linux-kernel

Hi,

This series fixes client reference counting for fw_iso_resource_auto
objects, reported by Dingisoul[1].

The issue is caused by ad-hoc client reference counting when scheduling a
work item to handle the iso_resource_auto client resource. This series
holds a client reference for the lifetime of the fw_iso_resource_auto
object instead.

[1] https://sourceforge.net/p/linux1394/mailman/message/59317811/

Takashi Sakamoto (2):
  firewire: cdev: hold client reference for fw_iso_resource_auto
    lifetime
  firewire: cdev: fix client refcount leak in iso_resource_auto_work()

 drivers/firewire/core-cdev.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)


base-commit: 4eea44fefc15929ac2de10bd9b12319ea42e0e0f
-- 
2.53.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] firewire: cdev: hold client reference for fw_iso_resource_auto lifetime
  2026-09-23 17:23 [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
@ 2026-09-23 17:23 ` Takashi Sakamoto
  2026-09-23 17:23 ` [PATCH 2/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
  2026-09-27  1:37 ` [PATCH 0/2] " Takashi Sakamoto
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2026-09-23 17:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: dingiso.kernel, linux-kernel

The fw_iso_resource_auto object can outlive the operation that created
it since hte main operation for it is done in a work item. This work and
release paths access members of the client structure, therefore the
client structure must remain valid for the lifetime of an
fw_iso_resource_auto object.

Hold a reference to the client structure for the lifetime of the
fw_iso_resource_auto object.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-cdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 50419f10b04f..9964c66f2989 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1406,6 +1406,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 			// xarray and prepare for deletion, unless the client is shutting down.
 			scoped_guard(spinlock_irq,  &client->lock) {
 				if (!client->in_shutdown && xa_erase(&client->resource_xa, index)) {
+					// For the incrementation by add_client_resource().
 					client_put(client);
 					free = true;
 				}
@@ -1445,6 +1446,9 @@ static void iso_resource_auto_work(struct work_struct *work)
 		kfree(r->e_alloc);
 		kfree(r->e_dealloc);
 		kfree(r);
+
+		// For the incrementation by ioctl_allocate_iso_resource().
+		client_put(client);
 	}
  out:
 	client_put(client);
@@ -1491,6 +1495,7 @@ static int ioctl_allocate_iso_resource(struct client *client, union ioctl_arg *a
 	if (err < 0)
 		return err;
 	request->handle = r->resource.handle;
+	client_get(client);
 
 	retain_and_null_ptr(e1);
 	retain_and_null_ptr(e2);
-- 
2.53.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work()
  2026-09-23 17:23 [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
  2026-09-23 17:23 ` [PATCH 1/2] firewire: cdev: hold client reference for fw_iso_resource_auto lifetime Takashi Sakamoto
@ 2026-09-23 17:23 ` Takashi Sakamoto
  2026-09-27  1:37 ` [PATCH 0/2] " Takashi Sakamoto
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2026-09-23 17:23 UTC (permalink / raw)
  To: linux1394-devel; +Cc: dingiso.kernel, linux-kernel

The client reference leaks when the pending work is cancelled because an
additional reference was taken when scheduling the work.

The reference held by the fw_iso_resource_auto object already ensures
that the client structure remains valid for the lifetime of the object.
Therefore, the additional reference taken when scheduling the work is
unnecessary.

Remove the additional reference.

Reported-by: Dingisoul <dingiso.kernel@gmail.com>
Link: https://sourceforge.net/p/linux1394/mailman/message/59317811/
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-cdev.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 9964c66f2989..a626468b4b0f 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -196,9 +196,7 @@ static int is_outbound_transaction_resource(const struct client_resource *resour
 
 static void schedule_iso_resource_auto(struct iso_resource_auto *r, unsigned long delay)
 {
-	client_get(r->client);
-	if (!queue_delayed_work(fw_workqueue, &r->work, delay))
-		client_put(r->client);
+	queue_delayed_work(fw_workqueue, &r->work, delay);
 }
 
 /*
@@ -1369,13 +1367,13 @@ static void iso_resource_auto_work(struct work_struct *work)
 		// Allow 1000ms grace period for other reallocations.
 		if (time_is_after_jiffies64(reset_jiffies + secs_to_jiffies(1))) {
 			schedule_iso_resource_auto(r, msecs_to_jiffies(333));
-			goto out;
+			return;
 		}
 		break;
 	case ISO_RES_AUTO_REALLOC:
 		// We could be called twice within the same generation.
 		if (resource_generation == current_generation)
-			goto out;
+			return;
 		break;
 	case ISO_RES_AUTO_DEALLOC:
 	default:
@@ -1397,7 +1395,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 		// Is this generation outdated already?  As long as this resource sticks in the
 		// xarray, it will be scheduled again for a newer generation or at shutdown.
 		if (channel == -EAGAIN)
-			goto out;
+			return;
 
 		bool success = channel >= 0 || bandwidth > 0;
 
@@ -1415,7 +1413,7 @@ static void iso_resource_auto_work(struct work_struct *work)
 
 		if (todo == ISO_RES_AUTO_REALLOC) {
 			if (success)
-				goto out;
+				return;
 
 			// Notify the userspace client of the failure through a deallocation event.
 			e = r->e_dealloc;
@@ -1450,8 +1448,6 @@ static void iso_resource_auto_work(struct work_struct *work)
 		// For the incrementation by ioctl_allocate_iso_resource().
 		client_put(client);
 	}
- out:
-	client_put(client);
 }
 
 static void release_iso_resource_auto(struct client *client, struct client_resource *resource)
-- 
2.53.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work()
  2026-09-23 17:23 [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
  2026-09-23 17:23 ` [PATCH 1/2] firewire: cdev: hold client reference for fw_iso_resource_auto lifetime Takashi Sakamoto
  2026-09-23 17:23 ` [PATCH 2/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
@ 2026-09-27  1:37 ` Takashi Sakamoto
  2 siblings, 0 replies; 4+ messages in thread
From: Takashi Sakamoto @ 2026-09-27  1:37 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel, dingiso.kernel

On Thu, Sep 24, 2026 at 02:23:26AM +0900, Takashi Sakamoto wrote:
> Hi,
> 
> This series fixes client reference counting for fw_iso_resource_auto
> objects, reported by Dingisoul[1].
> 
> The issue is caused by ad-hoc client reference counting when scheduling a
> work item to handle the iso_resource_auto client resource. This series
> holds a client reference for the lifetime of the fw_iso_resource_auto
> object instead.
> 
> [1] https://sourceforge.net/p/linux1394/mailman/message/59317811/
> 
> Takashi Sakamoto (2):
>   firewire: cdev: hold client reference for fw_iso_resource_auto
>     lifetime
>   firewire: cdev: fix client refcount leak in iso_resource_auto_work()
> 
>  drivers/firewire/core-cdev.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-27  1:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23 17:23 [PATCH 0/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
2026-09-23 17:23 ` [PATCH 1/2] firewire: cdev: hold client reference for fw_iso_resource_auto lifetime Takashi Sakamoto
2026-09-23 17:23 ` [PATCH 2/2] firewire: cdev: fix client refcount leak in iso_resource_auto_work() Takashi Sakamoto
2026-09-27  1:37 ` [PATCH 0/2] " Takashi Sakamoto

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®