[struct buf] Unlocked access to b_vflags?
Alexander Lochmann
alexander.lochmann at tu-dortmund.de
Tue Apr 13 11:44:18 UTC 2021
Thx!
Do you mind adding "Found by LockDoc" to the Commit-Msg?
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
-------------- 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/0851733a/attachment.sig>
More information about the freebsd-fs
mailing list