netdev - Re: [tipc-discussion] [PATCH] tipc: fix a lockdep warning

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] [day] [month] [year] [list]

Date:	Thu, 5 Dec 2013 16:56:54 +0800
From:	Ying Xue <ying.xue@...driver.com>
To:	Jason HU <huzhijiang@...il.com>
CC:	wangweidong <wangweidong1@...wei.com>,
	"<jon.maloy@...csson.com>" <jon.maloy@...csson.com>,
	"<allan.stephens@...driver.com>" <allan.stephens@...driver.com>,
	"<davem@...emloft.net>" <davem@...emloft.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"tipc-discussion@...ts.sourceforge.net" 
	<tipc-discussion@...ts.sourceforge.net>,
	"dingtianhong@...wei.com" <dingtianhong@...wei.com>
Subject: Re: [tipc-discussion] [PATCH] tipc: fix a lockdep warning


>>> diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
>>> index e0d0805..ab8f96c 100644
>>> --- a/net/tipc/name_distr.c
>>> +++ b/net/tipc/name_distr.c
>>> @@ -138,7 +138,9 @@ static void named_cluster_distribute(struct sk_buff *buf)
>>>            if (!buf_copy)
>>>                break;
>>>            msg_set_destnode(buf_msg(buf_copy), n_ptr->addr);
>>> +            write_unlock_bh(&tipc_nametbl_lock);
>>>            tipc_link_send(buf_copy, n_ptr->addr, n_ptr->addr);
>>> +            write_lock_bh(&tipc_nametbl_lock);
>>
>> We cannot temporarily release/hold tipc_nametbl_lock here, please see
>> below call path:
>>
>> tipc_nametbl_withdraw()
>>  tipc_named_withdraw()
>>    named_cluster_distribute()
>>      tipc_link_send()
>>
>> Especially in tipc_nametbl_withdraw(), we must hold tipc_nametbl_lock to
>> protect name table before tipc_named_withdraw() is called. If we
>> temporarily release tipc_nametbl_lock in named_cluster_distribute(), I
>> am afraid that name table might be changed by another thread at the
>> moment, having name table inconsistent possibly.
> 
> If the inconsistent possibility that you worried about is the matter of mutually exclusive between user threads which call bind(), then I think it will not occur, because we still got port lock hold when calling tipc_nametbl_publish(). So even we do not have to use tipc_nametbl_lock to warp up both tipc_nametbl_insert_publ() and tipc_name_publish() . Just tipc_nametbl_insert_publ() is enough. May be I missed something, please correct me.
> 

Actually now the key point for us is not to discuss whether the patch is
right or not, but consider whether the patch is necessary or not. As I
emphasized below, tipc_net_lock will be quickly removed completely. Even
if the patch is correct now, we have to revert it after the removal of
tipc_net_lock because:

1. the patch reported warning will automatically disappear if
tipc_net_lcok is removed.

2. I believe no anyone likes what the patch is doing.

Regards,
Ying

> 
> 
>>
>> Actually we are implementing another patchset purging the tipc_net_lock
>> from TIPC stack. If the patchset is involved, I guess the issue would
>> disappear.
>>
>> If you have an interesting to see how to purge to tipc_net_lock, please
>> monitor tipc-discussion mail list.
>>
>> Regards,
>> Ying
>>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html