Date: Fri, 17 Aug 2018 09:01:32 -0700 From: Conrad Meyer <cem@freebsd.org> To: "Andrey V. Elsukov" <bu7cher@yandex.ru> 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: <CAG6CVpX3Ath8JXBTvsaM2OxD%2B4JeAxCe040SyFnqYwk-1_SpuQ@mail.gmail.com> In-Reply-To: <6d1d2b23-978b-af1d-4022-16d09c9a42f5@yandex.ru> References: <201807180056.w6I0uPb6000705@repo.freebsd.org> <6d1d2b23-978b-af1d-4022-16d09c9a42f5@yandex.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Please file a PR and we can track it there.
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 obje=
cts
in it =E2=80=94 so drop the list lock over xf_init(). It does not appear t=
hat
xform_init can race with xform_detach with the lock dropped.
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).
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).
I think this patch may be safe, and would remove the OOM-induced
deadlock condition:
--- 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);
}
Best,
Conrad
On Fri, Aug 17, 2018 at 6:01 AM, Andrey V. Elsukov <bu7cher@yandex.ru> wrot=
e:
> On 18.07.2018 03:56, Conrad Meyer wrote:
>> Author: cem
>> Date: Wed Jul 18 00:56:25 2018
>> New Revision: 336439
>> URL: https://svnweb.freebsd.org/changeset/base/336439
>>
>> Log:
>> OpenCrypto: Convert sessions to opaque handles instead of integers
>>
>> Track session objects in the framework, and pass handles between the
>> framework (OCF), consumers, and drivers. Avoid redundancy and complex=
ity in
>> individual drivers by allocating session memory in the framework and
>> providing it to drivers in ::newsession().
>
> Hi,
>
> this produces WITNESS warning, since crypto_newsession() allocates
> memory with M_WAITOK flag while xform_init() holds mutex:
>
> uma_zalloc_arg: zone "crypto_session" with the following non-sleepable
> locks held:
> exclusive sleep mutex xforms_list (IPsec transforms list) r =3D 0
> (0xffffffff81fdb840) locked @
> /home/devel/freebsd/base/head/sys/netipsec/key.c:8676
> stack backtrace:
> #0 0xffffffff80c01643 at witness_debugger+0x73
> #1 0xffffffff80c02a21 at witness_warn+0x461
> #2 0xffffffff80ed98a8 at uma_zalloc_arg+0x38
> #3 0xffffffff80e4a0ca at crypto_newsession+0x1ea
> #4 0xffffffff80e3994c at esp_init+0x37c
> #5 0xffffffff80e31e68 at key_setsaval+0x7f8
> #6 0xffffffff80e307e2 at key_newsav+0x302
> #7 0xffffffff80e2ba0d at key_add+0x53d
> #8 0xffffffff80e263f5 at key_parse+0xac5
> #9 0xffffffff80c34dc7 at sosend_generic+0x447
> #10 0xffffffff80c34ffd at sosend+0x6d
> #11 0xffffffff80c3c170 at kern_sendit+0x240
> #12 0xffffffff80c3c4be at sendit+0x19e
> #13 0xffffffff80c3c30d at sys_sendto+0x4d
> #14 0xffffffff8107e9c1 at amd64_syscall+0x281
> #15 0xffffffff8105846d at fast_syscall_common+0x101
>
> --
> WBR, Andrey V. Elsukov
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAG6CVpX3Ath8JXBTvsaM2OxD%2B4JeAxCe040SyFnqYwk-1_SpuQ>