Date: Tue, 18 Sep 2018 14:56:53 +0300 From: "Andrey V. Elsukov" <bu7cher@yandex.ru> To: cem@freebsd.org Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r336439 - in head: share/man/man9 sys/crypto/aesni sys/crypto/armv8 sys/crypto/blake2 sys/crypto/ccp sys/crypto/via sys/dev/cesa sys/dev/cxgbe/crypto sys/dev/hifn sys/dev/safe sys/dev/s... Message-ID: <f4fe2e9c-a054-0e7f-afb9-7aad8b3549b0@yandex.ru> In-Reply-To: <CAG6CVpX3Ath8JXBTvsaM2OxD%2B4JeAxCe040SyFnqYwk-1_SpuQ@mail.gmail.com> References: <201807180056.w6I0uPb6000705@repo.freebsd.org> <6d1d2b23-978b-af1d-4022-16d09c9a42f5@yandex.ru> <CAG6CVpX3Ath8JXBTvsaM2OxD%2B4JeAxCe040SyFnqYwk-1_SpuQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--5FqrIpHqBPZoOAtu9ddhXkpe3QMcqY9jP
Content-Type: multipart/mixed; boundary="NUGoJLdKsOiUkR24bZRU2Hs1iDY49Jh0U";
protected-headers="v1"
From: "Andrey V. Elsukov" <bu7cher@yandex.ru>
To: cem@freebsd.org
Cc: src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org,
svn-src-head@freebsd.org
Message-ID: <f4fe2e9c-a054-0e7f-afb9-7aad8b3549b0@yandex.ru>
Subject: Re: svn commit: r336439 - in head: share/man/man9 sys/crypto/aesni
sys/crypto/armv8 sys/crypto/blake2 sys/crypto/ccp sys/crypto/via sys/dev/cesa
sys/dev/cxgbe/crypto sys/dev/hifn sys/dev/safe sys/dev/s...
References: <201807180056.w6I0uPb6000705@repo.freebsd.org>
<6d1d2b23-978b-af1d-4022-16d09c9a42f5@yandex.ru>
<CAG6CVpX3Ath8JXBTvsaM2OxD+4JeAxCe040SyFnqYwk-1_SpuQ@mail.gmail.com>
In-Reply-To: <CAG6CVpX3Ath8JXBTvsaM2OxD+4JeAxCe040SyFnqYwk-1_SpuQ@mail.gmail.com>
--NUGoJLdKsOiUkR24bZRU2Hs1iDY49Jh0U
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable
On 17.08.2018 19:01, Conrad Meyer wrote:
> Please file a PR and we can track it there.
>=20
> The first suggestion that comes to mind is that the XFORMS_LOCK
> protects modification of the xforms list =E2=80=94 not the lifetime of =
objects
> in it =E2=80=94 so drop the list lock over xf_init(). It does not appe=
ar that
> xform_init can race with xform_detach with the lock dropped.
>=20
> xform_init is called only by key_setsaval, which is called in two
> places: key_newsav, and key_update. In key_newsav, it is used on
> sav's that are not yet linked in to a sah. In key_update, it is on
> larbal sav's only (i.e., linked in to savtree_larval list but not
> savtree_alive list).
>=20
> xform_detach -> key_delete_xform only enumerates over the sahtree
> looking for sah's with sav's present in savtree_alive with matching
> xform. Since neither key_newsav nor key_update insert the sav into
> the sah savtree_alive list until after setsaval -> xform_init, there
> is no race between xform_init and xform_detach (protected by
> SAHTREE_WLOCK).
>=20
> I think this patch may be safe, and would remove the OOM-induced
> deadlock condition:
>=20
> --- netipsec/key.c (revision 337955)
> +++ netipsec/key.c (working copy)
> @@ -8676,11 +8676,13 @@
> XFORMS_LOCK();
> LIST_FOREACH(entry, &xforms, chain) {
> if (entry->xf_type =3D=3D xftype) {
> + XFORMS_UNLOCK();
> ret =3D (*entry->xf_init)(sav, entry);
> - break;
> + goto out;
> }
> }
> XFORMS_UNLOCK();
> +out:
> return (ret);
> }
I think this will work for now, but a potential problem can occur when
xform that is going to be detached placed is some kld. I.e. some
application invokes SA creation and another thread does kldunload
ipsec_esp.ko (currently we don't have such module, but...)
So, when we drop XFORMS_LOCK after check "entry->xf_type =3D=3D xftype",
xf_init can become unavailable due to kldunload happened.
--=20
WBR, Andrey V. Elsukov
--NUGoJLdKsOiUkR24bZRU2Hs1iDY49Jh0U--
--5FqrIpHqBPZoOAtu9ddhXkpe3QMcqY9jP
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
iQEzBAEBCAAdFiEE5lkeG0HaFRbwybwAAcXqBBDIoXoFAlug6AYACgkQAcXqBBDI
oXpddQf+NzoYGl7fBofowvy2N9nCKB0hrIjQZVb1cYm/OtZlCxJFbaEr3AEhZyez
hcJzYIt3WAdz2sUR+Mz2qnG0f5Nrcxn6737lHCm981Ra+gZqYhSvZDMeeFHsTCs6
QIPrVPVXa0OFFfLiARW35ZmjcRAUhEaRUnHmgTFzz+DUUiK59jWBkuTLAipX0uET
7WESwGHS3PPj2o5xx2TsxD96nrCmGFgYQfUBL9TPLa+DzdzQoSwdTM9HDseM7M3K
D7yOK6kca310ehxUN5yfKhok9Bmkz/MOkCjF2r2gvAOEcKxru1mcinTGreffzNXh
Q7O6EstitP19WwOpsAc0MsVoiD5L/A==
=1DJX
-----END PGP SIGNATURE-----
--5FqrIpHqBPZoOAtu9ddhXkpe3QMcqY9jP--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f4fe2e9c-a054-0e7f-afb9-7aad8b3549b0>