From: Takashi Sakamoto <o-takashi@sakamocchi.jp> To: tiwai@suse.de Cc: linux1394-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, Niels Dossche <dossche.niels@gmail.com>, stable@vger.kernel.org Subject: [PATCH 3/3] firewire: core: extend card->lock in fw_core_handle_bus_reset Date: Sat, 9 Apr 2022 13:12:43 +0900 [thread overview] Message-ID: <20220409041243.603210-4-o-takashi@sakamocchi.jp> (raw) In-Reply-To: <20220409041243.603210-1-o-takashi@sakamocchi.jp> From: Niels Dossche <dossche.niels@gmail.com> card->local_node and card->bm_retries are both always accessed under card->lock. fw_core_handle_bus_reset has a check whose condition depends on card->local_node and whose body writes to card->bm_retries. Both of these accesses are not under card->lock. Move the lock acquiring of card->lock to before this check such that these accesses do happen when card->lock is held. fw_destroy_nodes is called inside the check. Since fw_destroy_nodes already acquires card->lock inside its function body, move this out to the callsites of fw_destroy_nodes. Also add a comment to indicate which locking is necessary when calling fw_destroy_nodes. Cc: <stable@vger.kernel.org> Signed-off-by: Niels Dossche <dossche.niels@gmail.com> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- drivers/firewire/core-card.c | 3 +++ drivers/firewire/core-topology.c | 9 +++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 54be88167c60..f3b3953cac83 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -668,6 +668,7 @@ EXPORT_SYMBOL_GPL(fw_card_release); void fw_core_remove_card(struct fw_card *card) { struct fw_card_driver dummy_driver = dummy_driver_template; + unsigned long flags; card->driver->update_phy_reg(card, 4, PHY_LINK_ACTIVE | PHY_CONTENDER, 0); @@ -682,7 +683,9 @@ void fw_core_remove_card(struct fw_card *card) dummy_driver.stop_iso = card->driver->stop_iso; card->driver = &dummy_driver; + spin_lock_irqsave(&card->lock, flags); fw_destroy_nodes(card); + spin_unlock_irqrestore(&card->lock, flags); /* Wait for all users, especially device workqueue jobs, to finish. */ fw_card_put(card); diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index b63d55f5ebd3..f40c81534381 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -375,16 +375,13 @@ static void report_found_node(struct fw_card *card, card->bm_retries = 0; } +/* Must be called with card->lock held */ void fw_destroy_nodes(struct fw_card *card) { - unsigned long flags; - - spin_lock_irqsave(&card->lock, flags); card->color++; if (card->local_node != NULL) for_each_fw_node(card, card->local_node, report_lost_node); card->local_node = NULL; - spin_unlock_irqrestore(&card->lock, flags); } static void move_tree(struct fw_node *node0, struct fw_node *node1, int port) @@ -510,6 +507,8 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, struct fw_node *local_node; unsigned long flags; + spin_lock_irqsave(&card->lock, flags); + /* * If the selfID buffer is not the immediate successor of the * previously processed one, we cannot reliably compare the @@ -521,8 +520,6 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, card->bm_retries = 0; } - spin_lock_irqsave(&card->lock, flags); - card->broadcast_channel_allocated = card->broadcast_channel_auto_allocated; card->node_id = node_id; /* -- 2.34.1
WARNING: multiple messages have this Message-ID
From: Takashi Sakamoto <o-takashi@sakamocchi.jp> To: tiwai@suse.de Cc: alsa-devel@alsa-project.org, linux1394-devel@lists.sourceforge.net, Niels Dossche <dossche.niels@gmail.com>, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH 3/3] firewire: core: extend card->lock in fw_core_handle_bus_reset Date: Sat, 9 Apr 2022 13:12:43 +0900 [thread overview] Message-ID: <20220409041243.603210-4-o-takashi@sakamocchi.jp> (raw) In-Reply-To: <20220409041243.603210-1-o-takashi@sakamocchi.jp> From: Niels Dossche <dossche.niels@gmail.com> card->local_node and card->bm_retries are both always accessed under card->lock. fw_core_handle_bus_reset has a check whose condition depends on card->local_node and whose body writes to card->bm_retries. Both of these accesses are not under card->lock. Move the lock acquiring of card->lock to before this check such that these accesses do happen when card->lock is held. fw_destroy_nodes is called inside the check. Since fw_destroy_nodes already acquires card->lock inside its function body, move this out to the callsites of fw_destroy_nodes. Also add a comment to indicate which locking is necessary when calling fw_destroy_nodes. Cc: <stable@vger.kernel.org> Signed-off-by: Niels Dossche <dossche.niels@gmail.com> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- drivers/firewire/core-card.c | 3 +++ drivers/firewire/core-topology.c | 9 +++------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/firewire/core-card.c b/drivers/firewire/core-card.c index 54be88167c60..f3b3953cac83 100644 --- a/drivers/firewire/core-card.c +++ b/drivers/firewire/core-card.c @@ -668,6 +668,7 @@ EXPORT_SYMBOL_GPL(fw_card_release); void fw_core_remove_card(struct fw_card *card) { struct fw_card_driver dummy_driver = dummy_driver_template; + unsigned long flags; card->driver->update_phy_reg(card, 4, PHY_LINK_ACTIVE | PHY_CONTENDER, 0); @@ -682,7 +683,9 @@ void fw_core_remove_card(struct fw_card *card) dummy_driver.stop_iso = card->driver->stop_iso; card->driver = &dummy_driver; + spin_lock_irqsave(&card->lock, flags); fw_destroy_nodes(card); + spin_unlock_irqrestore(&card->lock, flags); /* Wait for all users, especially device workqueue jobs, to finish. */ fw_card_put(card); diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c index b63d55f5ebd3..f40c81534381 100644 --- a/drivers/firewire/core-topology.c +++ b/drivers/firewire/core-topology.c @@ -375,16 +375,13 @@ static void report_found_node(struct fw_card *card, card->bm_retries = 0; } +/* Must be called with card->lock held */ void fw_destroy_nodes(struct fw_card *card) { - unsigned long flags; - - spin_lock_irqsave(&card->lock, flags); card->color++; if (card->local_node != NULL) for_each_fw_node(card, card->local_node, report_lost_node); card->local_node = NULL; - spin_unlock_irqrestore(&card->lock, flags); } static void move_tree(struct fw_node *node0, struct fw_node *node1, int port) @@ -510,6 +507,8 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, struct fw_node *local_node; unsigned long flags; + spin_lock_irqsave(&card->lock, flags); + /* * If the selfID buffer is not the immediate successor of the * previously processed one, we cannot reliably compare the @@ -521,8 +520,6 @@ void fw_core_handle_bus_reset(struct fw_card *card, int node_id, int generation, card->bm_retries = 0; } - spin_lock_irqsave(&card->lock, flags); - card->broadcast_channel_allocated = card->broadcast_channel_auto_allocated; card->node_id = node_id; /* -- 2.34.1
next prev parent reply other threads:[~2022-04-09 4:13 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-04-09 4:12 [PATCH 0/3] firewire: fixes for kernel v4.9 or later Takashi Sakamoto 2022-04-09 4:12 ` Takashi Sakamoto 2022-04-09 4:12 ` [PATCH 1/3] firewire: fix potential uaf in outbound_phy_packet_callback() Takashi Sakamoto 2022-04-09 4:12 ` Takashi Sakamoto 2022-04-09 4:12 ` [PATCH 2/3] firewire: remove check of list iterator against head past the loop body Takashi Sakamoto 2022-04-09 4:12 ` Takashi Sakamoto 2022-04-09 4:12 ` Takashi Sakamoto [this message] 2022-04-09 4:12 ` [PATCH 3/3] firewire: core: extend card->lock in fw_core_handle_bus_reset Takashi Sakamoto 2022-04-25 6:05 ` [PATCH 0/3] firewire: fixes for kernel v4.9 or later Takashi Iwai 2022-04-25 6:05 ` Takashi Iwai
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220409041243.603210-4-o-takashi@sakamocchi.jp \ --to=o-takashi@sakamocchi.jp \ --cc=alsa-devel@alsa-project.org \ --cc=dossche.niels@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux1394-devel@lists.sourceforge.net \ --cc=stable@vger.kernel.org \ --cc=tiwai@suse.de \ --subject='Re: [PATCH 3/3] firewire: core: extend card->lock in fw_core_handle_bus_reset' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.