Date: Tue, 13 Apr 2021 13:44:14 +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: <ad235e06-ec65-bd0f-e665-fde25dc35cf1@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)
--AmhoOYiNmm1rPl3BVA7mX8Pd6hdZG3E9A
Content-Type: multipart/mixed; boundary="4jgzcgYTERslCHmALQ79z7QVANtJcJVrI";
protected-headers="v1"
From: Alexander Lochmann <alexander.lochmann@tu-dortmund.de>
To: Konstantin Belousov <kostikbel@gmail.com>
Cc: freebsd-fs@freebsd.org
Message-ID: <ad235e06-ec65-bd0f-e665-fde25dc35cf1@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>
--4jgzcgYTERslCHmALQ79z7QVANtJcJVrI
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: de-DE
Content-Transfer-Encoding: quoted-printable
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 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
--4jgzcgYTERslCHmALQ79z7QVANtJcJVrI--
--AmhoOYiNmm1rPl3BVA7mX8Pd6hdZG3E9A
Content-Type: application/pgp-signature; name="OpenPGP_signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="OpenPGP_signature"
-----BEGIN PGP SIGNATURE-----
wsF5BAABCAAjFiEElhZsUHzVP0dbkjCRWT7tBbw+9v0FAmB1hA4FAwAAAAAACgkQWT7tBbw+9v1h
Zg//RSp7wM6yeF/eLw5Z7nFW+cCiK6wPheiURqJmMzlYbZRsceqrtROJmvH4cqmNsHBO9l40Byqu
Iy2PFJc3PpmDScS0JiT6P94+oGP3vPVX+qkOAiiuBfQeHnjc5XUUNycndEI7Q6+jfCY+kmMKvMQa
P4WGf47v0EFrQVZFlMy4yZ7oatDzC/QtLznFz7OYBOeTsjd5QlUXiWj3sAhap/09VZ0+GOGmsgPU
Je4HQZpq+fu85uzuv8kieX+Ww/uQXO1auFKZQ1NJVBMQwgQKyyXCQcszqPUoAjv1xCrT4OGkHKLQ
SZKLzphfvNbFGa9spL6cwXGAa64NrdnNeHzmDc0ohhLEZ2hBHUPgv4YktK/xCPjwtPSw1PrrtQWV
L/npNoLYefhWTPLLQEEhCbaiEW38qbupvv2+kMUO7UC8P8s8YGPUgsqtuQRQFCs3smprcjwCYKaR
SC92X6Dbp7H8DRju3Tz2wiRpixvMKol6wEfOHgqfG9IPfGP3Cq9YNIDQ2SwQoFXONu9E+/u/J2H6
MZ9x5RtVhJMFBfyZEVO3ENUc0/37LO2zU3R/14zm3TIuK6PJ4DYXDCS3s72kGDiHOneC2CYWxnF/
2x6kVjyzJBpUOPap4rEtYkOxK5bBsGgeOxw8mdroLu2DTwt/dY/ZCJwbDMKDjhZv1nW0Ym3gkKmE
cpc=
=wjdN
-----END PGP SIGNATURE-----
--AmhoOYiNmm1rPl3BVA7mX8Pd6hdZG3E9A--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ad235e06-ec65-bd0f-e665-fde25dc35cf1>