[struct buf] Unlocked access to b_vflags?

[Alexander Lochmann]

Out of curiosity: What's the path from function entry to the access in 
line 759?
The lock is acquired upon function entry in line 7197, and never 
released afterwards (except for the lines 7212 and 7217).

- 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/20210419/2a1ca665/attachment.sig>