linux-kernel - Re: [Ocfs2-devel] [PATCH 2/2] ocfs2: fix deadlocks when taking inode lock at vfs entry points

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <836cc2c3-f00e-adf2-922e-d4d7a8b3286d@gmail.com>
Date:   Thu, 12 Jan 2017 19:36:27 +0800
From:   Joseph Qi <jiangqi903@...il.com>
To:     Eric Ren <zren@...e.com>
Cc:     ocfs2-devel@....oracle.com, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joel Becker <jlbec@...lplan.org>, Gang He <ghe@...e.com>,
        Junxiao Bi <junxiao.bi@...cle.com>, mfasheh@...sity.com
Subject: Re: [Ocfs2-devel] [PATCH 2/2] ocfs2: fix deadlocks when taking inode
 lock at vfs entry points


On 17/1/12 19:24, Eric Ren wrote:
> Hi Joseph,
>
> On 01/09/2017 10:13 AM, Eric Ren wrote:
>>>>> So you are trying to fix it by making phase3 finish without really 
>>>>> doing
>>>> Phase3 can go ahead because this node is already under protection 
>>>> of cluster lock.
>>> You said it was blocked...
>> Oh, sorry, I meant phase3 can go ahead if this patch set is applied;-)
>>
>>> "Another hand, the recursive cluster lock (the second one) will be 
>>> blocked in
>>> __ocfs2_cluster_lock() because of OCFS2_LOCK_BLOCKED."
>>>>> __ocfs2_cluster_lock, then Process B can continue either.
>>>>> Let us bear in mind that phase1 and phase3 are in the same context 
>>>>> and
>>>>> executed in order. That's why I think there is no need to check if 
>>>>> locked
>>>>> by myself in phase1.
>> Sorry, I still cannot see it. Without keeping track of the first 
>> cluster lock, how can we
>> know if
>> we are under a context that has already been in the protecting of 
>> cluster lock? How can we
>> handle
>> the recursive locking (the second cluster lock) if we don't have this 
>> information?
>>>>> If phase1 finds it is already locked by myself, that means the holder
>>>>> is left by last operation without dec holder. That's why I think 
>>>>> it is a bug
>>>>> instead of a recursive lock case.
>> I think I got your point here. Do you mean that we should just add 
>> the lock holder at the
>> first locking position
>> without checking before that? Unfortunately, it's tricky here to know 
>> exactly which ocfs2
>> routine will be the first vfs
>> entry point, such as ocfs2_get_acl() which can be both the first vfs 
>> entry point and the
>> second vfs entry point after
>> ocfs2_permission(), right?
>>
>> It will be a coding bug if the problem you concern about happens. I 
>> think we don't need to
>> worry about this much because
>> the code logic here is quite simple;-)
> Ping...
>
> Did I clear your doubts by the last email? I really want to get your 
> point, if not.
>
> If there's any problem, I will fix them in the next version;-)
Yes, but I still worry about the code bug case will be hidden behind 
recursive lock...
Anyway, It depends on others...

Thanks,
Joseph
>
> Thanks,
> Eric
>
>>
>> Thanks for your patience!
>> Eric
>>
>>>> D
>