[struct buf] Unlocked access to b_vflags?
Alexander Lochmann
alexander.lochmann at tu-dortmund.de
Tue Apr 13 13:08:15 UTC 2021
On 13.04.21 14:35, Konstantin Belousov wrote:
> On Tue, Apr 13, 2021 at 02:13:57PM +0200, Alexander Lochmann wrote:
>> "The bug was found by analyzing the memory accesses to certain data types
>> and lock operations while running a load. The whole workflow is called
>> LockDoc."
>> or
>> "The bug was found by performing lock analysis using LockDoc."
>> Is this enough, or shall I explain it in more detail?
>>
>> Unfortunately, we do not have a project page yet.
>> I, however, have the link to our paper:
>> https://doi.org/10.1145/3302424.3303948
> But it is beyond the paywall?
Oh. Yes, it is.
I put a version of the paper on my personal website:
https://ess.cs.tu-dortmund.de/Staff/al/Publications/index.html
I don't know how long this website will be available after I leave
university. :(
- Alex
>
>>
>> - Alex
>>
>> On 13.04.21 13:55, Konstantin Belousov wrote:
>>> On Tue, Apr 13, 2021 at 01:44:14PM +0200, Alexander Lochmann wrote:
>>>> Thx!
>>>> Do you mind adding "Found by LockDoc" to the Commit-Msg?
>>> I do not, but I have no idea what LockDoc is. Can you provide the complete
>>> attribution sentence, perhaps including the URL to the relevant web page?
>>>
>>>>
>>>> Cheers,
>>>> Alex
>>>>
>>>> On 13.04.21 12:25, Konstantin Belousov wrote:
>>>>> On Mon, Apr 12, 2021 at 11:19:05PM +0200, Alexander Lochmann wrote:
>>>>>> Hi folks,
>>>>>>
>>>>>> I'm was digging through our data set when I encountered a strange situation:
>>>>>> According to the code in trunc_dependencies() in sys/ufs/ffs/ffs_softdep.c,
>>>>>> the bo_lock should be held. At least that's how I read the code.
>>>>>> However, we see several thousands of accesses to b_vflags without the
>>>>>> bo_lock held.
>>>>>> At least the own b_lock is acquired.
>>>>>> The access happens in line 7549: bp->b_vflags |= BV_SCANNED; [1]
>>>>>> Can you please shed some light on this situation?
>>>>>> Is the b_lock sufficeint, and somehow overrules the bo_lock?
>>>>>> Am I missing something?
>>>>> I think you found a valid race. There is one more place where BV_SCANNED
>>>>> was manipulated without owning bufobj lock. Patch below should fix
both.
>>>>>
>>>>> commit a678470b1307542c5a46b930c119b2358863e0d2
>>>>> Author: Konstantin Belousov <kib at FreeBSD.org>
>>>>> Date: Tue Apr 13 13:22:56 2021 +0300
>>>>>
>>>>> b_vflags update requries bufobj lock
>>>>> Reported by: Alexander Lochmann <alexander.lochmann at tu-dortmund.de> (trunc_dependencies())
>>>>>
>>>>> diff --git a/sys/ufs/ffs/ffs_softdep.c b/sys/ufs/ffs/ffs_softdep.c
>>>>> index 0091b5dcd3b8..23c0cf6e128b 100644
>>>>> --- a/sys/ufs/ffs/ffs_softdep.c
>>>>> +++ b/sys/ufs/ffs/ffs_softdep.c
>>>>> @@ -7546,7 +7546,9 @@ trunc_dependencies(ip, freeblks, lastlbn, lastoff, flags)
>>>>> BO_LOCK(bo);
>>>>> goto cleanrestart;
>>>>> }
>>>>> + BO_LOCK(bo);
>>>>> bp->b_vflags |= BV_SCANNED;
>>>>> + BO_UNLOCK(bo);
>>>>> bremfree(bp);
>>>>> if (blkoff != 0) {
>>>>> allocbuf(bp, blkoff);
>>>>> diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c
>>>>> index dc638595eb7b..05eb19c0ee13 100644
>>>>> --- a/sys/ufs/ffs/ffs_vnops.c
>>>>> +++ b/sys/ufs/ffs/ffs_vnops.c
>>>>> @@ -321,8 +321,9 @@ ffs_syncvnode(struct vnode *vp, int waitfor, int flags)
>>>>> if (BUF_LOCK(bp,
>>>>> LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK,
>>>>> BO_LOCKPTR(bo)) != 0) {
>>>>> + BO_LOCK(bo);
>>>>> bp->b_vflags &= ~BV_SCANNED;
>>>>> - goto next;
>>>>> + goto next_locked;
>>>>> }
>>>>> } else
>>>>> continue;
>>>>> @@ -385,6 +386,7 @@ ffs_syncvnode(struct vnode *vp, int waitfor, int flags)
>>>>> * to start from a known point.
>>>>> */
>>>>> BO_LOCK(bo);
>>>>> +next_locked:
>>>>> nbp = TAILQ_FIRST(&bo->bo_dirty.bv_hd);
>>>>> }
>>>>> if (waitfor != MNT_WAIT) {
>>>>>
>>>>
>>>> --
>>>> Technische Universität Dortmund
>>>> Alexander Lochmann PGP key: 0xBC3EF6FD
>>>> Otto-Hahn-Str. 16 phone: +49.231.7556141
>>>> D-44227 Dortmund fax: +49.231.7556116
>>>> http://ess.cs.tu-dortmund.de/Staff/al
>>>>
>>>
>>>
>>>
>>
>> --
>> Technische Universität Dortmund
>> Alexander Lochmann PGP key: 0xBC3EF6FD
>> Otto-Hahn-Str. 16 phone: +49.231.7556141
>> D-44227 Dortmund fax: +49.231.7556116
>> http://ess.cs.tu-dortmund.de/Staff/al
>>
>
>
>
--
Technische Universität Dortmund
Alexander Lochmann PGP key: 0xBC3EF6FD
Otto-Hahn-Str. 16 phone: +49.231.7556141
D-44227 Dortmund fax: +49.231.7556116
http://ess.cs.tu-dortmund.de/Staff/al
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20210413/c6505cb6/attachment.sig>
More information about the freebsd-fs
mailing list