Date: Mon, 19 Apr 2021 21:43:47 +0200 From: Alexander Lochmann <alexander.lochmann@tu-dortmund.de> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-fs@freebsd.org Subject: Re: [struct buf] Unlocked access to b_vflags? Message-ID: <4ade0f5d-d4f4-616a-b198-fc58f947070d@tu-dortmund.de> In-Reply-To: <YHVxfMrU9lmw3sG9@kib.kiev.ua> References: <792c8a3d-8ea6-073f-3fda-b3eb793ef2b9@tu-dortmund.de> <YHVxfMrU9lmw3sG9@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--MRvfc5qisZWaZafXXSPKFbyRmKvisJENe
Content-Type: multipart/mixed; boundary="A3ypNaQRaCga4xnx721QhScM0k7JIFCph";
protected-headers="v1"
From: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
To: Konstantin Belousov <kostikbel@gmail.com>
Cc: freebsd-fs@freebsd.org
Message-ID: <4ade0f5d-d4f4-616a-b198-fc58f947070d@tu-dortmund.de>
Subject: Re: [struct buf] Unlocked access to b_vflags?
References: <792c8a3d-8ea6-073f-3fda-b3eb793ef2b9@tu-dortmund.de>
<YHVxfMrU9lmw3sG9@kib.kiev.ua>
In-Reply-To: <YHVxfMrU9lmw3sG9@kib.kiev.ua>
--A3ypNaQRaCga4xnx721QhScM0k7JIFCph
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: de-DE
Content-Transfer-Encoding: quoted-printable
Out of curiosity: What's the path from function entry to the access in=20
line 759?
The lock is acquired upon function entry in line 7197, and never=20
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 situ=
ation:
>> According to the code in trunc_dependencies() in sys/ufs/ffs/ffs_softd=
ep.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 |=3D 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_SCANN=
ED
> was manipulated without owning bufobj lock. Patch below should fix bot=
h.
>=20
> commit a678470b1307542c5a46b930c119b2358863e0d2
> Author: Konstantin Belousov <kib@FreeBSD.org>
> Date: Tue Apr 13 13:22:56 2021 +0300
>=20
> b_vflags update requries bufobj lock
> =20
> Reported by: Alexander Lochmann <alexander.lochmann@tu-dortmund=
=2Ede> (trunc_dependencies())
>=20
> 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 |=3D BV_SCANNED;
> + BO_UNLOCK(bo);
> bremfree(bp);
> if (blkoff !=3D 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 fl=
ags)
> if (BUF_LOCK(bp,
> LK_EXCLUSIVE | LK_SLEEPFAIL | LK_INTERLOCK,
> BO_LOCKPTR(bo)) !=3D 0) {
> + BO_LOCK(bo);
> bp->b_vflags &=3D ~BV_SCANNED;
> - goto next;
> + goto next_locked;
> }
> } else
> continue;
> @@ -385,6 +386,7 @@ ffs_syncvnode(struct vnode *vp, int waitfor, int fl=
ags)
> * to start from a known point.
> */
> BO_LOCK(bo);
> +next_locked:
> nbp =3D TAILQ_FIRST(&bo->bo_dirty.bv_hd);
> }
> if (waitfor !=3D MNT_WAIT) {
>=20
--=20
Technische Universit=C3=A4t 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
--A3ypNaQRaCga4xnx721QhScM0k7JIFCph--
--MRvfc5qisZWaZafXXSPKFbyRmKvisJENe
Content-Type: application/pgp-signature; name="OpenPGP_signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="OpenPGP_signature"
-----BEGIN PGP SIGNATURE-----
wsF5BAABCAAjFiEElhZsUHzVP0dbkjCRWT7tBbw+9v0FAmB93XMFAwAAAAAACgkQWT7tBbw+9v2Z
RhAAhOy5JpO/uLgFsTuW6e+QpeeIdWJIhppqC1FB6wdL4kUrqcYqTUwMs4CsGL3SGHE5JvGPRjKu
2i8i3aZ6pBnWhjloS1ZRJ8miZ6Jk1ChX4ZXfEOzxh4RAC7JKbiLw6I6Eb8u6k3pDBp8InPMpTaBi
V2ase+KOYYjZlVVDmByWZybPweEJD85iWInSlrMpcnCamsA5cvJwXbu2eIBCtMHAQWXVfi55agmU
fTEVileE7Sm4kW+ohQEC/U/h2hFgtMbvD3k6mn8UYiLKquU8ZG1AusyOCCXcPa/REk7nE13y/YYx
1Z0W0lT0+lHUix9E1wj5Bku7TAh3GUCn83A3bzHI2j851RG6+OL7s83hTGScL4l9mKdR8xjv6CUd
B5k3HGMyfDlex9usiiwDHFhjWEIKt7wZbJYFxTmeZ9+wgtYwAHsGSzhr2uo6Lr4FU2uqFyb33GVz
B2T2v+BzpUg9yVBXEkJqessQ/tECLRg4lQbQZSVFIfKeLWntV4sETgMXlzEfv8jK37RhoehHH6kr
tMmb03fx6DrSN8sSiTsmvfSHKk0iS3fEHnRiNPGZbEiRXJ2cTv2VvZrEN3w+dp+icRUqHQAI33Nz
ZATqnxkK08OaYRiWMNi6JCdOHCeRHsw90SJWvC49jIv0LT0vpkb/iVl7TtKrN6YAygY1ZErtyJcB
hhA=
=BdVe
-----END PGP SIGNATURE-----
--MRvfc5qisZWaZafXXSPKFbyRmKvisJENe--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4ade0f5d-d4f4-616a-b198-fc58f947070d>