mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis
@ 2026-11-02  4:19 Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 1/9] firewire: core: add context analysis annotations for address handler Takashi Sakamoto
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Hi,

This series prepares the cdev layer for compiler-based context analysis
by adding the necessary annotations.

It also adds a __counted_by() annotation as well as cleanup functions to
simplify resource allocation and error handling.

Thanks

Takashi Sakamoto (9):
  firewire: core: add context analysis annotations for address handler
  firewire: cdev: add context analysis annotations for phy_receiver_list
  firewire: cdev: add context analysis annotations for isochronous
    context
  firewire: cdev: annotate data length in inbound_transaction_resource
  firewire: core: add context analysis annotation to
    queue_bus_reset_event()
  firewire: cdev: add context analysis annotations for client shutdown
    flag and event list
  firewire: cdev: use cleanup function for outbound transaction event
    allocation
  firewire: cdev: use cleanup function for descriptor_resource
    allocation
  firewire: cdev: enable compiler-based context analysis

 drivers/firewire/Makefile           |  1 +
 drivers/firewire/core-cdev.c        | 87 +++++++++++++++--------------
 drivers/firewire/core-transaction.c | 12 ++--
 3 files changed, 55 insertions(+), 45 deletions(-)


base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.53.0


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

* [PATCH 1/9] firewire: core: add context analysis annotations for address handler
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 2/9] firewire: cdev: add context analysis annotations for phy_receiver_list Takashi Sakamoto
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The address handler list is protected by a spinlock when handlers are
added to or removed from the list, while readers traverse the list within
RCU read-side critical sections.

Use both the spinlock and RCU read-side lock annotations to describe the
synchronization requirements for accessing the list. This allows
compiler-based context analysis to verify that the list is accessed under
the appropriate synchronization context.

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

diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 0ebe70a4d440..8d7e0f03aa6a 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -590,12 +590,18 @@ void fw_send_phy_config(struct fw_card *card,
 	wait_for_completion_timeout(&phy_config_done, timeout);
 }
 
+static DEFINE_SPINLOCK(address_handler_list_lock);
+static __guarded_by(RCU, &address_handler_list_lock) LIST_HEAD(address_handler_list);
+
 static struct fw_address_handler *lookup_overlapping_address_handler(
 	struct list_head *list, unsigned long long offset, size_t length)
+__must_hold(address_handler_list_lock)
 {
 	struct fw_address_handler *handler;
 
-	list_for_each_entry_rcu(handler, list, link) {
+	lockdep_assert_held(&address_handler_list_lock);
+
+	list_for_each_entry(handler, list, link) {
 		if (handler->offset < offset + length &&
 		    offset < handler->offset + handler->length)
 			return handler;
@@ -613,6 +619,7 @@ static bool is_enclosing_handler(struct fw_address_handler *handler,
 
 static struct fw_address_handler *lookup_enclosing_address_handler(
 	struct list_head *list, unsigned long long offset, size_t length)
+__must_hold_shared(RCU)
 {
 	struct fw_address_handler *handler;
 
@@ -624,9 +631,6 @@ static struct fw_address_handler *lookup_enclosing_address_handler(
 	return NULL;
 }
 
-static DEFINE_SPINLOCK(address_handler_list_lock);
-static LIST_HEAD(address_handler_list);
-
 /**
  * var fw_high_memory_region - An address range for high memory.
  *
-- 
2.53.0


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

* [PATCH 2/9] firewire: cdev: add context analysis annotations for phy_receiver_list
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 1/9] firewire: core: add context analysis annotations for address handler Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context Takashi Sakamoto
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The phy receiver list is protected by a mutex.

Use guard annotations for both list and entry to describe the
synchronization requirements for accessing them.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 87efd65e6b00..512afa589e2e 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -48,7 +48,7 @@
 #define FW_CDEV_VERSION_EVENT_ASYNC_TSTAMP	6
 
 static DEFINE_MUTEX(phy_receiver_list_mutex);
-static LIST_HEAD(phy_receiver_list);
+static LIST_HEAD_GUARDED(phy_receiver_list, phy_receiver_list_mutex);
 
 struct client {
 	u32 version;
@@ -68,7 +68,7 @@ struct client {
 	struct fw_iso_buffer buffer;
 	unsigned long vm_start;
 
-	struct list_head phy_receiver_link;
+	struct list_head phy_receiver_link __guarded_by(phy_receiver_list_mutex);
 	u64 phy_receiver_closure;
 
 	struct list_head link;
@@ -320,7 +320,7 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
 	INIT_LIST_HEAD(&client->event_list);
 	init_waitqueue_head(&client->wait);
 	init_waitqueue_head(&client->tx_flush_wait);
-	INIT_LIST_HEAD(&client->phy_receiver_link);
+	context_unsafe(INIT_LIST_HEAD(&client->phy_receiver_link));
 	INIT_LIST_HEAD(&client->link);
 	kref_init(&client->kref);
 	mutex_init(&client->iso_context_mutex);
-- 
2.53.0


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

* [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 1/9] firewire: core: add context analysis annotations for address handler Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 2/9] firewire: cdev: add context analysis annotations for phy_receiver_list Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02 10:40   ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 4/9] firewire: cdev: annotate data length in inbound_transaction_resource Takashi Sakamoto
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

A client can operate on a single isochronous context at a time. A mutex
is used to ensure exclusive access to the context.

Use guard annotations to describe the synchronization requirements for
accessing the context.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 512afa589e2e..418b72cb8835 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -62,7 +62,7 @@ struct client {
 	wait_queue_head_t tx_flush_wait;
 	u64 bus_reset_closure;
 
-	struct fw_iso_context *iso_context;
+	struct fw_iso_context *iso_context __pt_guarded_by(iso_context_mutex);
 	struct mutex iso_context_mutex;
 	u64 iso_closure;
 	struct fw_iso_buffer buffer;
@@ -1129,8 +1129,11 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
 static int ioctl_set_iso_channels(struct client *client, union ioctl_arg *arg)
 {
 	struct fw_cdev_set_iso_channels *a = &arg->set_iso_channels;
-	struct fw_iso_context *ctx = client->iso_context;
+	struct fw_iso_context *ctx;
 
+	guard(mutex)(&client->iso_context_mutex);
+
+	ctx = client->iso_context;
 	if (ctx == NULL || a->handle != 0)
 		return -EINVAL;
 
@@ -1149,12 +1152,15 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
 {
 	struct fw_cdev_queue_iso *a = &arg->queue_iso;
 	struct fw_cdev_iso_packet __user *p, *end, *next;
-	struct fw_iso_context *ctx = client->iso_context;
+	struct fw_iso_context *ctx;
 	unsigned long payload, buffer_end, transmit_header_bytes = 0;
 	u32 control;
 	int count;
 	DEFINE_RAW_FLEX(struct fw_iso_packet, u, header, 64);
 
+	guard(mutex)(&client->iso_context_mutex);
+
+	ctx = client->iso_context;
 	if (ctx == NULL || a->handle != 0)
 		return -EINVAL;
 
@@ -1252,6 +1258,8 @@ static int ioctl_start_iso(struct client *client, union ioctl_arg *arg)
 	    FW_CDEV_ISO_CONTEXT_MATCH_TAG3 != FW_ISO_CONTEXT_MATCH_TAG3 ||
 	    FW_CDEV_ISO_CONTEXT_MATCH_ALL_TAGS != FW_ISO_CONTEXT_MATCH_ALL_TAGS);
 
+	guard(mutex)(&client->iso_context_mutex);
+
 	if (client->iso_context == NULL || a->handle != 0)
 		return -EINVAL;
 
@@ -1267,6 +1275,8 @@ static int ioctl_stop_iso(struct client *client, union ioctl_arg *arg)
 {
 	struct fw_cdev_stop_iso *a = &arg->stop_iso;
 
+	guard(mutex)(&client->iso_context_mutex);
+
 	if (client->iso_context == NULL || a->handle != 0)
 		return -EINVAL;
 
@@ -1277,6 +1287,8 @@ static int ioctl_flush_iso(struct client *client, union ioctl_arg *arg)
 {
 	struct fw_cdev_flush_iso *a = &arg->flush_iso;
 
+	guard(mutex)(&client->iso_context_mutex);
+
 	if (client->iso_context == NULL || a->handle != 0)
 		return -EINVAL;
 
@@ -1937,8 +1949,10 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
 	scoped_guard(mutex, &client->device->client_list_mutex)
 		list_del(&client->link);
 
-	if (client->iso_context)
-		fw_iso_context_destroy(client->iso_context);
+	scoped_guard(mutex, &client->iso_context_mutex) {
+		if (client->iso_context)
+			fw_iso_context_destroy(client->iso_context);
+	}
 	mutex_destroy(&client->iso_context_mutex);
 
 	if (client->buffer.pages)
-- 
2.53.0


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

* [PATCH 4/9] firewire: cdev: annotate data length in inbound_transaction_resource
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 5/9] firewire: core: add context analysis annotation to queue_bus_reset_event() Takashi Sakamoto
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The inbound_transaction_resource structure stores the length of its data
in a dedicated member, which can be used with the __counted_by()
annotatnion.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 418b72cb8835..cbf61dc06971 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -118,7 +118,7 @@ struct inbound_transaction_resource {
 	struct fw_card *card;
 	struct fw_request *request;
 	bool is_fcp;
-	void *data;
+	void *data __counted_by_ptr(length);
 	size_t length;
 };
 
-- 
2.53.0


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

* [PATCH 5/9] firewire: core: add context analysis annotation to queue_bus_reset_event()
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 4/9] firewire: cdev: annotate data length in inbound_transaction_resource Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 6/9] firewire: cdev: add context analysis annotations for client shutdown flag and event list Takashi Sakamoto
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The caller of fill_bus_reset_event() must hold the client list mutex. This
requirement is also satisfied when the function is called from
queue_bus_reset_event() via fw_device_cdev_update(), but compiler-based
context analysis reports a warning because queue_bus_reset_event() lacks
the corresponding context annotation.

Add the annotation to queue_bus_reset_event().

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index cbf61dc06971..927363d03846 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -443,6 +443,7 @@ static void for_each_client(struct fw_device *device,
 }
 
 static void queue_bus_reset_event(struct client *client)
+__must_hold(&client->device->client_list_mutex)
 {
 	struct bus_reset_event *e;
 	struct client_resource *resource;
-- 
2.53.0


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

* [PATCH 6/9] firewire: cdev: add context analysis annotations for client shutdown flag and event list
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 5/9] firewire: core: add context analysis annotation to queue_bus_reset_event() Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 7/9] firewire: cdev: use cleanup function for outbound transaction event allocation Takashi Sakamoto
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The client structure contains a shutdown flag and an event list, both of
which are accessed while holding a mutex.

Add guard annotations to describe the synchronization requirements for
accessing these members.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 927363d03846..da1c761f8b73 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -55,9 +55,9 @@ struct client {
 	struct fw_device *device;
 
 	struct mutex mutex;
-	bool in_shutdown;
+	bool in_shutdown __guarded_by(mutex);
 	struct xarray resource_xa;
-	struct list_head event_list;
+	struct list_head event_list __guarded_by(mutex);
 	wait_queue_head_t wait;
 	wait_queue_head_t tx_flush_wait;
 	u64 bus_reset_closure;
@@ -317,7 +317,7 @@ static int fw_device_op_open(struct inode *inode, struct file *file)
 	client->device = device;
 	mutex_init(&client->mutex);
 	xa_init_flags(&client->resource_xa, XA_FLAGS_ALLOC1 | XA_FLAGS_LOCK_BH);
-	INIT_LIST_HEAD(&client->event_list);
+	context_unsafe(INIT_LIST_HEAD(&client->event_list));
 	init_waitqueue_head(&client->wait);
 	init_waitqueue_head(&client->tx_flush_wait);
 	context_unsafe(INIT_LIST_HEAD(&client->phy_receiver_link));
@@ -1971,8 +1971,10 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
 	}
 	xa_destroy(&client->resource_xa);
 
-	list_for_each_entry_safe(event, next_event, &client->event_list, link)
-		kfree(event);
+	scoped_guard(mutex, &client->mutex) {
+		list_for_each_entry_safe(event, next_event, &client->event_list, link)
+			kfree(event);
+	}
 
 	client_put(client);
 
@@ -1988,7 +1990,8 @@ static __poll_t fw_device_op_poll(struct file *file, poll_table * pt)
 
 	if (fw_device_is_shutdown(client->device))
 		mask |= EPOLLHUP | EPOLLERR;
-	if (!list_empty(&client->event_list))
+
+	if (context_unsafe(!list_empty(&client->event_list)))
 		mask |= EPOLLIN | EPOLLRDNORM;
 
 	return mask;
-- 
2.53.0


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

* [PATCH 7/9] firewire: cdev: use cleanup function for outbound transaction event allocation
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 6/9] firewire: cdev: add context analysis annotations for client shutdown flag and event list Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 8/9] firewire: cdev: use cleanup function for descriptor_resource allocation Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 9/9] firewire: cdev: enable compiler-based context analysis Takashi Sakamoto
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Use a cleanup function to simplify error handling when allocating an
outbound transaction event.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index da1c761f8b73..f94b22a608e9 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -672,9 +672,8 @@ static int init_request(struct client *client,
 			struct fw_cdev_send_request *request,
 			int destination_id, int speed)
 {
-	struct outbound_transaction_event *e;
+	struct outbound_transaction_event *e __free(kfree) = NULL;
 	void *payload;
-	int ret;
 
 	if (request->tcode != TCODE_STREAM_DATA &&
 	    (request->length > 4096 || request->length > 512 << speed))
@@ -705,24 +704,19 @@ static int init_request(struct client *client,
 		payload = rsp->data;
 	}
 
-	if (request->data && copy_from_user(payload, u64_to_uptr(request->data), request->length)) {
-		ret = -EFAULT;
-		goto failed;
-	}
+	if (request->data && copy_from_user(payload, u64_to_uptr(request->data), request->length))
+		return -EFAULT;
 
-	ret = add_client_resource(client, &e->r.resource, release_transaction);
+	int ret = add_client_resource(client, &e->r.resource, release_transaction);
 	if (ret < 0)
-		goto failed;
+		return ret;
 
 	fw_send_request_with_tstamp(client->device->card, &e->r.transaction, request->tcode,
 				    destination_id, request->generation, speed, request->offset,
 				    payload, request->length, complete_transaction, e);
-	return 0;
-
- failed:
-	kfree(e);
+	retain_and_null_ptr(e);
 
-	return ret;
+	return 0;
 }
 
 static int ioctl_send_request(struct client *client, union ioctl_arg *arg)
-- 
2.53.0


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

* [PATCH 8/9] firewire: cdev: use cleanup function for descriptor_resource allocation
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (6 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 7/9] firewire: cdev: use cleanup function for outbound transaction event allocation Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  2026-11-02  4:19 ` [PATCH 9/9] firewire: cdev: enable compiler-based context analysis Takashi Sakamoto
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

Use a cleanup function to simplify error handling when allocating a
descriptor_resource object.

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

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index f94b22a608e9..ef4fca7e8363 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -960,7 +960,7 @@ static void release_descriptor(struct client *client,
 static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 {
 	struct fw_cdev_add_descriptor *a = &arg->add_descriptor;
-	struct descriptor_resource *r;
+	struct descriptor_resource *r __free(kfree) = NULL;
 	int ret;
 
 	/* Access policy: Allow this ioctl only on local nodes' device files. */
@@ -974,11 +974,8 @@ static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 	if (r == NULL)
 		return -ENOMEM;
 
-	if (copy_from_user(r->data, u64_to_uptr(a->data),
-			   flex_array_size(r, data, a->length))) {
-		ret = -EFAULT;
-		goto failed;
-	}
+	if (copy_from_user(r->data, u64_to_uptr(a->data), flex_array_size(r, data, a->length)))
+		return -EFAULT;
 
 	r->descriptor.length    = a->length;
 	r->descriptor.immediate = a->immediate;
@@ -987,20 +984,16 @@ static int ioctl_add_descriptor(struct client *client, union ioctl_arg *arg)
 
 	ret = fw_core_add_descriptor(&r->descriptor);
 	if (ret < 0)
-		goto failed;
+		return ret;
 
 	ret = add_client_resource(client, &r->resource, release_descriptor);
 	if (ret < 0) {
 		fw_core_remove_descriptor(&r->descriptor);
-		goto failed;
+		return ret;
 	}
-	a->handle = r->resource.handle;
+	a->handle = no_free_ptr(r)->resource.handle;
 
 	return 0;
- failed:
-	kfree(r);
-
-	return ret;
 }
 
 static int ioctl_remove_descriptor(struct client *client, union ioctl_arg *arg)
-- 
2.53.0


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

* [PATCH 9/9] firewire: cdev: enable compiler-based context analysis
  2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
                   ` (7 preceding siblings ...)
  2026-11-02  4:19 ` [PATCH 8/9] firewire: cdev: use cleanup function for descriptor_resource allocation Takashi Sakamoto
@ 2026-11-02  4:19 ` Takashi Sakamoto
  8 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02  4:19 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

The kernel is transitioning from Sparse-based context analysis to
compiler-based context analysis, which requires LLVM/Clang 23 or later.
Individual compilation units can opt in to this analysis through their
Makefiles.

Enable compiler-based context analysis for the cdev layer.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firewire/Makefile b/drivers/firewire/Makefile
index e27fca37b4b2..0e5e7027f22e 100644
--- a/drivers/firewire/Makefile
+++ b/drivers/firewire/Makefile
@@ -4,6 +4,7 @@
 #
 
 CONTEXT_ANALYSIS_core-transaction.o := y
+CONTEXT_ANALYSIS_core-cdev.o := y
 
 firewire-core-y += core-trace.o core-card.o core-cdev.o core-device.o \
                    core-iso.o core-topology.o core-transaction.o
-- 
2.53.0


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

* Re: [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context
  2026-11-02  4:19 ` [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context Takashi Sakamoto
@ 2026-11-02 10:40   ` Takashi Sakamoto
  0 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2026-11-02 10:40 UTC (permalink / raw)
  To: linux1394-devel; +Cc: linux-kernel

On Mon, Nov 02, 2026 at 01:19:07PM +0900, Takashi Sakamoto wrote:
> A client can operate on a single isochronous context at a time. A mutex
> is used to ensure exclusive access to the context.
> 
> Use guard annotations to describe the synchronization requirements for
> accessing the context.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  drivers/firewire/core-cdev.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
> index 512afa589e2e..418b72cb8835 100644
> --- a/drivers/firewire/core-cdev.c
> +++ b/drivers/firewire/core-cdev.c
> @@ -62,7 +62,7 @@ struct client {
>  	wait_queue_head_t tx_flush_wait;
>  	u64 bus_reset_closure;
>  
> -	struct fw_iso_context *iso_context;
> +	struct fw_iso_context *iso_context __pt_guarded_by(iso_context_mutex);
>  	struct mutex iso_context_mutex;
>  	u64 iso_closure;
>  	struct fw_iso_buffer buffer;
> @@ -1129,8 +1129,11 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
>  static int ioctl_set_iso_channels(struct client *client, union ioctl_arg *arg)
>  {
>  	struct fw_cdev_set_iso_channels *a = &arg->set_iso_channels;
> -	struct fw_iso_context *ctx = client->iso_context;
> +	struct fw_iso_context *ctx;
>  
> +	guard(mutex)(&client->iso_context_mutex);
> +
> +	ctx = client->iso_context;
>  	if (ctx == NULL || a->handle != 0)
>  		return -EINVAL;
>  
> @@ -1149,12 +1152,15 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
>  {
>  	struct fw_cdev_queue_iso *a = &arg->queue_iso;
>  	struct fw_cdev_iso_packet __user *p, *end, *next;
> -	struct fw_iso_context *ctx = client->iso_context;
> +	struct fw_iso_context *ctx;
>  	unsigned long payload, buffer_end, transmit_header_bytes = 0;
>  	u32 control;
>  	int count;
>  	DEFINE_RAW_FLEX(struct fw_iso_packet, u, header, 64);
>  
> +	guard(mutex)(&client->iso_context_mutex);

Sashiko reports that get_user()/copy_from_user() under mutex locking can
cause A-B/B-A deadlock between the above mutex and mm->mmap_lock.

https://sashiko.dev/#/patchset/20261102041913.1187814-1-o-takashi%40sakamocchi.jp

Let me rework for the series.

> +	ctx = client->iso_context;
>  	if (ctx == NULL || a->handle != 0)
>  		return -EINVAL;
>
> @@ -1252,6 +1258,8 @@ static int ioctl_start_iso(struct client *client, union ioctl_arg *arg)
>  	    FW_CDEV_ISO_CONTEXT_MATCH_TAG3 != FW_ISO_CONTEXT_MATCH_TAG3 ||
>  	    FW_CDEV_ISO_CONTEXT_MATCH_ALL_TAGS != FW_ISO_CONTEXT_MATCH_ALL_TAGS);
>  
> +	guard(mutex)(&client->iso_context_mutex);
> +
>  	if (client->iso_context == NULL || a->handle != 0)
>  		return -EINVAL;
>  
> @@ -1267,6 +1275,8 @@ static int ioctl_stop_iso(struct client *client, union ioctl_arg *arg)
>  {
>  	struct fw_cdev_stop_iso *a = &arg->stop_iso;
>  
> +	guard(mutex)(&client->iso_context_mutex);
> +
>  	if (client->iso_context == NULL || a->handle != 0)
>  		return -EINVAL;
>  
> @@ -1277,6 +1287,8 @@ static int ioctl_flush_iso(struct client *client, union ioctl_arg *arg)
>  {
>  	struct fw_cdev_flush_iso *a = &arg->flush_iso;
>  
> +	guard(mutex)(&client->iso_context_mutex);
> +
>  	if (client->iso_context == NULL || a->handle != 0)
>  		return -EINVAL;
>  
> @@ -1937,8 +1949,10 @@ static int fw_device_op_release(struct inode *inode, struct file *file)
>  	scoped_guard(mutex, &client->device->client_list_mutex)
>  		list_del(&client->link);
>  
> -	if (client->iso_context)
> -		fw_iso_context_destroy(client->iso_context);
> +	scoped_guard(mutex, &client->iso_context_mutex) {
> +		if (client->iso_context)
> +			fw_iso_context_destroy(client->iso_context);
> +	}
>  	mutex_destroy(&client->iso_context_mutex);
>  
>  	if (client->buffer.pages)
> -- 
> 2.53.0

Regards

Takashi Sakamoto

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

end of thread, other threads:[~2026-11-02 10:40 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02  4:19 [PATCH 0/9] firewire: cdev: add annotations for compiler-based context analysis Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 1/9] firewire: core: add context analysis annotations for address handler Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 2/9] firewire: cdev: add context analysis annotations for phy_receiver_list Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 3/9] firewire: cdev: add context analysis annotations for isochronous context Takashi Sakamoto
2026-11-02 10:40   ` Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 4/9] firewire: cdev: annotate data length in inbound_transaction_resource Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 5/9] firewire: core: add context analysis annotation to queue_bus_reset_event() Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 6/9] firewire: cdev: add context analysis annotations for client shutdown flag and event list Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 7/9] firewire: cdev: use cleanup function for outbound transaction event allocation Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 8/9] firewire: cdev: use cleanup function for descriptor_resource allocation Takashi Sakamoto
2026-11-02  4:19 ` [PATCH 9/9] firewire: cdev: enable compiler-based context analysis 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®