netdev - RE: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in tipc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 23 Nov 2023 06:54:11 +0000
From: Tung Quang Nguyen <tung.q.nguyen@...tech.com.au>
To: xu <xu.xin.sc@...il.com>
CC: "davem@...emloft.net" <davem@...emloft.net>, "jmaloy@...hat.com"
	<jmaloy@...hat.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "tipc-discussion@...ts.sourceforge.net"
	<tipc-discussion@...ts.sourceforge.net>, "xu.xin16@....com.cn"
	<xu.xin16@....com.cn>, "ying.xue@...driver.com" <ying.xue@...driver.com>,
	"yang.yang29@....com.cn" <yang.yang29@....com.cn>
Subject: RE: [RFC PATCH] net/tipc: reduce tipc_node lock holding time in
 tipc_rcv

>>This patch is wrong. le->link and link status must be protected by node lock. See what happens if tipc_node_timeout() is called, and
>the link goes down:
>>tipc_node_timeout()
>>   tipc_node_link_down()
>>   {
>>      struct tipc_link *l = le->link;
>>      ...
>>      if (delete) {
>>         kfree(l);
>>         le->link = NULL;
>>      }
>>      ...
>>   }
>
>Happy to see your reply. But Why? 'delete' is false from tipc_node_timeout(). Refer to:
>https://elixir.bootlin.com/linux/v6.7-rc2/source/net/tipc/node.c#L844
I should have explained it clearly:
1/ link status must be protected.
tipc_node_timeout()
   tipc_node_link_down()
   {
      struct tipc_link *l = le->link;
   
      ...
     __tipc_node_link_down(); <-- link status is referred. 
      ...
     if (delete) {
        kfree(l);
        le->link = NULL;
     }
     ...
   }

__tipc_node_link_down()
{
    ...
   if (!l || tipc_link_is_reset(l)) <-- read link status
    ...
    tipc_link_reset(l); <--- this function will reset all things related to link.
}

2/ le->link must be protected.
bearer_disable()
{
   ...
   tipc_node_delete_links(net, bearer_id); <--- this will delete all links.
   ...
}

tipc_node_delete_links()
{
   ...
  tipc_node_link_down(n, bearer_id, true);
   ...
}