[PATCH] firewire: core: use release/acquire pair when replacing underlying driver on card removal

[PATCH] firewire: core: use release/acquire pair when replacing underlying driver on card removal

[Takashi Sakamoto]

When removing a card instance, the underlying driver is replaced to handle
closing operations. Without proper memory reordering, other CPUs may
observe the driver before it is fully initialized.

Use a release/acquire pair to ensure that the driver's initialization is
visible before the driver is accessed for close operations.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-card.c        | 16 ++++++++-------
 drivers/firewire/core-cdev.c        |  2 +-
 drivers/firewire/core-device.c      |  8 ++++----
 drivers/firewire/core-iso.c         | 32 +++++++++++++++++++++--------
 drivers/firewire/core-transaction.c | 20 +++++++++---------
 5 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c
index be6f18d67ece..83c29a487d86 100644
--- a/drivers/firewire/core-card.c
+++ b/drivers/firewire/core-card.c
@@ -153,8 +153,8 @@ static void update_config_roms(void)
 
 	list_for_each_entry (card, &card_list, link) {
 		generate_config_rom(card, tmp_config_rom);
-		card->driver->set_config_rom(card, tmp_config_rom,
-					     config_rom_length);
+		smp_load_acquire(&card->driver)->set_config_rom(card, tmp_config_rom,
+								config_rom_length);
 	}
 }
 
@@ -231,7 +231,7 @@ static int reset_bus(struct fw_card *card, bool short_reset)
 
 	trace_bus_reset_initiate(card->index, card->generation, short_reset);
 
-	return card->driver->update_phy_reg(card, reg, 0, bit);
+	return smp_load_acquire(&card->driver)->update_phy_reg(card, reg, 0, bit);
 }
 
 void fw_schedule_bus_reset(struct fw_card *card, bool delayed, bool short_reset)
@@ -660,7 +660,8 @@ int fw_card_add(struct fw_card *card, u32 max_receive, u32 link_speed, u64 guid,
 
 	scoped_guard(mutex, &card_mutex) {
 		generate_config_rom(card, tmp_config_rom);
-		ret = card->driver->enable(card, tmp_config_rom, config_rom_length);
+		ret = smp_load_acquire(&card->driver)->enable(card, tmp_config_rom,
+							      config_rom_length);
 		if (ret < 0) {
 			card->isoc_wq = NULL;
 			card->async_wq = NULL;
@@ -804,7 +805,8 @@ void fw_core_remove_card(struct fw_card *card)
 	dummy_driver.free_iso_context	= card->driver->free_iso_context;
 	dummy_driver.stop_iso		= card->driver->stop_iso;
 	dummy_driver.disable		= card->driver->disable;
-	card->driver = &dummy_driver;
+	// Ensure the entire contents of the driver are visible to other CPUs.
+	smp_store_release(&card->driver, &dummy_driver);
 
 	drain_workqueue(card->isoc_wq);
 	drain_workqueue(card->async_wq);
@@ -843,12 +845,12 @@ EXPORT_SYMBOL(fw_core_remove_card);
  */
 int fw_card_read_cycle_time(struct fw_card *card, u32 *cycle_time)
 {
-	if (card->driver->read_csr == dummy_read_csr)
+	if (smp_load_acquire(&card->driver)->read_csr == dummy_read_csr)
 		return -ENODEV;
 
 	// It's possible to switch to dummy driver between the above and the below. This is the best
 	// effort to return -ENODEV.
-	*cycle_time = card->driver->read_csr(card, CSR_CYCLE_TIME);
+	*cycle_time = smp_load_acquire(&card->driver)->read_csr(card, CSR_CYCLE_TIME);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(fw_card_read_cycle_time);
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 60067c5b1973..87efd65e6b00 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1731,7 +1731,7 @@ static int ioctl_send_phy_packet(struct client *client, union ioctl_arg *arg)
 	trace_async_phy_outbound_initiate((uintptr_t)&e->p, card->index, e->p.generation,
 					  e->p.header[1], e->p.header[2]);
 
-	card->driver->send_request(card, &e->p);
+	smp_load_acquire(&card->driver)->send_request(card, &e->p);
 
 	return 0;
 }
diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
index 4e79c57263ff..a68100d3da0b 100644
--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -261,14 +261,14 @@ EXPORT_SYMBOL(fw_bus_type);
 
 int fw_device_enable_phys_dma(struct fw_device *device)
 {
-	int generation = device->generation;
+	struct fw_card *card = device->card;
 
 	/* device->node_id, accessed below, must not be older than generation */
+	int generation = device->generation;
 	smp_rmb();
+	int node_id = device->node_id;
 
-	return device->card->driver->enable_phys_dma(device->card,
-						     device->node_id,
-						     generation);
+	return smp_load_acquire(&card->driver)->enable_phys_dma(device->card, node_id, generation);
 }
 EXPORT_SYMBOL(fw_device_enable_phys_dma);
 
diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index d2b40a1a565e..fd6cbbf79d0b 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -144,8 +144,8 @@ struct fw_iso_context *__fw_iso_context_create(struct fw_card *card, int type, i
 {
 	struct fw_iso_context *ctx;
 
-	ctx = card->driver->allocate_iso_context(card, type, channel, header_size,
-						 header_storage_size);
+	ctx = smp_load_acquire(&card->driver)->allocate_iso_context(card, type, channel,
+							header_size, header_storage_size);
 	if (IS_ERR(ctx))
 		return ctx;
 
@@ -173,7 +173,9 @@ void fw_iso_context_destroy(struct fw_iso_context *ctx)
 	trace_isoc_inbound_single_destroy(ctx);
 	trace_isoc_inbound_multiple_destroy(ctx);
 
-	ctx->card->driver->free_iso_context(ctx);
+	struct fw_card *card = ctx->card;
+
+	smp_load_acquire(&card->driver)->free_iso_context(ctx);
 }
 EXPORT_SYMBOL(fw_iso_context_destroy);
 
@@ -184,7 +186,9 @@ int fw_iso_context_start(struct fw_iso_context *ctx,
 	trace_isoc_inbound_single_start(ctx, cycle, sync, tags);
 	trace_isoc_inbound_multiple_start(ctx, cycle, sync, tags);
 
-	return ctx->card->driver->start_iso(ctx, cycle, sync, tags);
+	struct fw_card *card = ctx->card;
+
+	return smp_load_acquire(&card->driver)->start_iso(ctx, cycle, sync, tags);
 }
 EXPORT_SYMBOL(fw_iso_context_start);
 
@@ -192,7 +196,9 @@ int fw_iso_context_set_channels(struct fw_iso_context *ctx, u64 *channels)
 {
 	trace_isoc_inbound_multiple_channels(ctx, *channels);
 
-	return ctx->card->driver->set_iso_channels(ctx, channels);
+	struct fw_card *card = ctx->card;
+
+	return smp_load_acquire(&card->driver)->set_iso_channels(ctx, channels);
 }
 
 int fw_iso_context_queue(struct fw_iso_context *ctx,
@@ -204,7 +210,9 @@ int fw_iso_context_queue(struct fw_iso_context *ctx,
 	trace_isoc_inbound_single_queue(ctx, payload, packet);
 	trace_isoc_inbound_multiple_queue(ctx, payload, packet);
 
-	return ctx->card->driver->queue_iso(ctx, packet, buffer, payload);
+	struct fw_card *card = ctx->card;
+
+	return smp_load_acquire(&card->driver)->queue_iso(ctx, packet, buffer, payload);
 }
 EXPORT_SYMBOL(fw_iso_context_queue);
 
@@ -214,7 +222,9 @@ void fw_iso_context_queue_flush(struct fw_iso_context *ctx)
 	trace_isoc_inbound_single_flush(ctx);
 	trace_isoc_inbound_multiple_flush(ctx);
 
-	ctx->card->driver->flush_queue_iso(ctx);
+	struct fw_card *card = ctx->card;
+
+	smp_load_acquire(&card->driver)->flush_queue_iso(ctx);
 }
 EXPORT_SYMBOL(fw_iso_context_queue_flush);
 
@@ -248,7 +258,9 @@ int fw_iso_context_flush_completions(struct fw_iso_context *ctx)
 
 	disable_work_sync(&ctx->work);
 
-	err = ctx->card->driver->flush_iso_completions(ctx);
+	struct fw_card *card = ctx->card;
+
+	err = smp_load_acquire(&card->driver)->flush_iso_completions(ctx);
 
 	enable_work(&ctx->work);
 
@@ -270,7 +282,9 @@ int fw_iso_context_stop(struct fw_iso_context *ctx)
 	if (WARN_ON_ONCE(current_work() == &ctx->work))
 		return 0;
 
-	err = ctx->card->driver->stop_iso(ctx);
+	struct fw_card *card = ctx->card;
+
+	err = smp_load_acquire(&card->driver)->stop_iso(ctx);
 
 	cancel_work_sync(&ctx->work);
 
diff --git a/drivers/firewire/core-transaction.c b/drivers/firewire/core-transaction.c
index 48558857e726..c2050b23b7eb 100644
--- a/drivers/firewire/core-transaction.c
+++ b/drivers/firewire/core-transaction.c
@@ -129,7 +129,7 @@ __must_not_hold(&card->transactions.lock)
 
 	// Cancel the packet transmission if it's still queued. That will schedule the packet
 	// transmission callback which cancels the transaction.
-	if (card->driver->cancel_packet(card, &transaction->packet) == 0)
+	if (smp_load_acquire(&card->driver)->cancel_packet(card, &transaction->packet) == 0)
 		return 0;
 
 	// If the request packet has already been sent, we need to see if the transaction is still
@@ -458,7 +458,7 @@ __must_not_hold(&card->transactions.lock)
 					      t->packet.header, payload,
 					      tcode_is_read_request(tcode) ? 0 : length / 4);
 
-	card->driver->send_request(card, &t->packet);
+	smp_load_acquire(&card->driver)->send_request(card, &t->packet);
 }
 EXPORT_SYMBOL_GPL(__fw_send_request);
 
@@ -555,7 +555,7 @@ void fw_send_phy_config(struct fw_card *card,
 	}
 
 	if (gap_count == FW_PHY_CONFIG_CURRENT_GAP_COUNT) {
-		gap_count = card->driver->read_phy_reg(card, 1);
+		gap_count = smp_load_acquire(&card->driver)->read_phy_reg(card, 1);
 		if (gap_count < 0)
 			return;
 
@@ -578,7 +578,7 @@ void fw_send_phy_config(struct fw_card *card,
 					  phy_config_packet.generation, phy_config_packet.header[1],
 					  phy_config_packet.header[2]);
 
-	card->driver->send_request(card, &phy_config_packet);
+	smp_load_acquire(&card->driver)->send_request(card, &phy_config_packet);
 	wait_for_completion_timeout(&phy_config_done, timeout);
 }
 
@@ -987,7 +987,7 @@ void fw_send_response(struct fw_card *card,
 					       request->response.header, data,
 					       data ? data_length / 4 : 0);
 
-	card->driver->send_response(card, &request->response);
+	smp_load_acquire(&card->driver)->send_response(card, &request->response);
 }
 EXPORT_SYMBOL(fw_send_response);
 
@@ -1238,7 +1238,7 @@ __must_not_hold(&card->transactions.lock)
 	 * The response handler may be executed while the request handler
 	 * is still pending.  Cancel the request handler.
 	 */
-	card->driver->cancel_packet(card, &t->packet);
+	smp_load_acquire(&card->driver)->cancel_packet(card, &t->packet);
 
 	invoke_callback(t, rcode, p->timestamp, data, data_length);
 }
@@ -1357,17 +1357,17 @@ static void handle_registers(struct fw_card *card, struct fw_request *request,
 	case CSR_BUS_TIME:
 	case CSR_BUSY_TIMEOUT:
 		if (tcode == TCODE_READ_QUADLET_REQUEST)
-			*data = cpu_to_be32(card->driver->read_csr(card, reg));
+			*data = cpu_to_be32(smp_load_acquire(&card->driver)->read_csr(card, reg));
 		else if (tcode == TCODE_WRITE_QUADLET_REQUEST)
-			card->driver->write_csr(card, reg, be32_to_cpu(*data));
+			smp_load_acquire(&card->driver)->write_csr(card, reg, be32_to_cpu(*data));
 		else
 			rcode = RCODE_TYPE_ERROR;
 		break;
 
 	case CSR_RESET_START:
 		if (tcode == TCODE_WRITE_QUADLET_REQUEST)
-			card->driver->write_csr(card, CSR_STATE_CLEAR,
-						CSR_STATE_BIT_ABDICATE);
+			smp_load_acquire(&card->driver)->write_csr(card, CSR_STATE_CLEAR,
+								   CSR_STATE_BIT_ABDICATE);
 		else
 			rcode = RCODE_TYPE_ERROR;
 		break;

base-commit: 82d59eb7f5431771cfc6fb074b74f466cd4e3353
-- 
2.53.0

Re: [PATCH] firewire: core: use release/acquire pair when replacing underlying driver on card removal

[Takashi Sakamoto]

On Sat, Oct 17, 2026 at 05:17:20PM +0900, Takashi Sakamoto wrote:
> When removing a card instance, the underlying driver is replaced to handle
> closing operations. Without proper memory reordering, other CPUs may
> observe the driver before it is fully initialized.
> 
> Use a release/acquire pair to ensure that the driver's initialization is
> visible before the driver is accessed for close operations.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  drivers/firewire/core-card.c        | 16 ++++++++-------
>  drivers/firewire/core-cdev.c        |  2 +-
>  drivers/firewire/core-device.c      |  8 ++++----
>  drivers/firewire/core-iso.c         | 32 +++++++++++++++++++++--------
>  drivers/firewire/core-transaction.c | 20 +++++++++---------
>  5 files changed, 47 insertions(+), 31 deletions(-)

Applied to for-next branch.


Regards

Takashi Sakamoto