[PATCH] firewire: core: fix invalid port index for parent device

[PATCH] firewire: core: fix invalid port index for parent device

[Takashi Sakamoto]

In a commit 24b7f8e5cd65 ("firewire: core: use helper functions for self
ID sequence"), the enumeration over self ID sequence was refactored with
some helper functions with KUnit tests. These helper functions are
guaranteed to work expectedly by the KUnit tests, however their application
includes a mistake to assign invalid value to the index of port connected
to parent device.

This bug affects the case that any extra node devices which has three or
more ports are connected to 1394 OHCI controller. In the case, the path
to update the tree cache could hits WARN_ON(), and gets general protection
fault due to the access to invalid address computed by the invalid value.

This commit fixes the bug to assign correct port index.

Cc: stable@vger.kernel.org
Reported-by: Edmund Raile <edmund.raile@proton.me>
Closes: https://lore.kernel.org/lkml/8a9902a4ece9329af1e1e42f5fea76861f0bf0e8.camel@proton.me/
Fixes: 24b7f8e5cd65 ("firewire: core: use helper functions for self ID sequence")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 drivers/firewire/core-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firewire/core-topology.c b/drivers/firewire/core-topology.c
index 6adadb11962e..892b94cfd626 100644
--- a/drivers/firewire/core-topology.c
+++ b/drivers/firewire/core-topology.c
@@ -204,7 +204,7 @@ static struct fw_node *build_tree(struct fw_card *card, const u32 *sid, int self
 				// the node->ports array where the parent node should be.  Later,
 				// when we handle the parent node, we fix up the reference.
 				++parent_count;
-				node->color = i;
+				node->color = port_index;
 				break;
 
 			case PHY_PACKET_SELF_ID_PORT_STATUS_CHILD:
-- 
2.45.2

Re: [PATCH] firewire: core: fix invalid port index for parent device

[Takashi Sakamoto]

On Fri, Oct 25, 2024 at 12:41:37PM +0900, Takashi Sakamoto wrote:
> In a commit 24b7f8e5cd65 ("firewire: core: use helper functions for self
> ID sequence"), the enumeration over self ID sequence was refactored with
> some helper functions with KUnit tests. These helper functions are
> guaranteed to work expectedly by the KUnit tests, however their application
> includes a mistake to assign invalid value to the index of port connected
> to parent device.
> 
> This bug affects the case that any extra node devices which has three or
> more ports are connected to 1394 OHCI controller. In the case, the path
> to update the tree cache could hits WARN_ON(), and gets general protection
> fault due to the access to invalid address computed by the invalid value.
> 
> This commit fixes the bug to assign correct port index.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Edmund Raile <edmund.raile@proton.me>
> Closes: https://lore.kernel.org/lkml/8a9902a4ece9329af1e1e42f5fea76861f0bf0e8.camel@proton.me/
> Fixes: 24b7f8e5cd65 ("firewire: core: use helper functions for self ID sequence")
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  drivers/firewire/core-topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to for-linus branch.


Regards

Takashi Sakamoto

Re: [PATCH] firewire: core: fix invalid port index for parent device

[Edmund Raile]

> In a commit 24b7f8e5cd65 ("firewire: core: use helper functions for
> self
> ID sequence"), the enumeration over self ID sequence was refactored
> with
> some helper functions with KUnit tests. These helper functions are
> guaranteed to work expectedly by the KUnit tests, however their
> application
> includes a mistake to assign invalid value to the index of port
> connected
> to parent device.
> 
> This bug affects the case that any extra node devices which has three
> or
> more ports are connected to 1394 OHCI controller. In the case, the
> path
> to update the tree cache could hits WARN_ON(), and gets general
> protection
> fault due to the access to invalid address computed by the invalid
> value.
> 
> This commit fixes the bug to assign correct port index.
> 
> Cc: stable@vger.kernel.org
> Reported-by: Edmund Raile <edmund.raile@proton.me>
> Closes:
> https://lore.kernel.org/lkml/8a9902a4ece9329af1e1e42f5fea76861f0bf0e8.camel@proton.me/
> Fixes: 24b7f8e5cd65 ("firewire: core: use helper functions for self
> ID sequence")
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  drivers/firewire/core-topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/firewire/core-topology.c
> b/drivers/firewire/core-topology.c
> index 6adadb11962e..892b94cfd626 100644
> --- a/drivers/firewire/core-topology.c
> +++ b/drivers/firewire/core-topology.c
> @@ -204,7 +204,7 @@ static struct fw_node *build_tree(struct fw_card
> *card, const u32 *sid, int self
>   // the node->ports array where the parent node should be.  Later,
>   // when we handle the parent node, we fix up the reference.
>   ++parent_count;
> - node->color = i;
> + node->color = port_index;
>   break;
> 
>   case PHY_PACKET_SELF_ID_PORT_STATUS_CHILD:
> --
> 2.45.2
Your patch is identical to my original proposals except for the line
numbers.


I've been using it since, no issues.

Thank you!

Tested-by: Edmund Raile <edmund.raile@proton.me>