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...

[Andrey V. Elsukov]

On 17.08.2018 19:01, Conrad Meyer wrote:
> 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 — not the lifetime of objects
> in it — so drop the list lock over xf_init().  It does not appear that
> 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 == xftype) {
> +                   XFORMS_UNLOCK();
>                     ret = (*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 == xftype",
xf_init can become unavailable due to kldunload happened.

-- 
WBR, Andrey V. Elsukov

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 554 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20180918/c26b7ff8/attachment.sig>