[Bug 218597] [CRYPTODEV] spurious wakeup and synchronisation problem

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218597

--- Comment #3 from John Baldwin <jhb at FreeBSD.org> ---
There's certainly nothing to prevent multiple threads from issuing ioctls
against the same session.  (However, having 'error' in the cse is buggy, but it
should also just be removed as all the uses of it check the crp->crp_etype it
is copied from just before using it anyway.)

The problem is that CRYPTO_F_DONE isn't really safe for how the /dev/crypto
ioctl handlers are using it (and probably isn't safe to use at all and should
arguably be removed).  Instead, the sleeps need to be conditional on a flag
that the callback function itself sets.  Probably the right way to handle this
is to allocate a separate structure that crp_opaque points to that contains the
"done" flag and a pointer to the cse.  The callback would set the done flag
under the cse lock and then issue the wakeup, and the ioctl routines would
sleep on this done flag in the new structure.  Something like:

struct crypt_op_data {
    bool done;
    struct csession *cse;
}

crypto_dev_op()
{
    struct crypt_op_data *cod;

    ...
    cod = malloc(sizeof(*cod), M_WAITOK | M_ZERO);
    cod->cse = cse;
    ...
    crp->crp_opaque = cod;  /* not 'cse' anymore */
    ...
    crypto_dispatch();
    mtx_lock(&cse->lock);
    while (error == 0 && !cod->done)
       mtx_sleep(crp, &cse->lock, ...);
    mtx_unlock(&cse->lock);
    ...
    /* be sure to clear done if retrying */
}

cryptodev_cb()
{
    struct cryptop *crp = op;
    struct crypto_op_data *cod = crp->crp_opaque;

    mtx_lock(&cod->cse->lock);
    cod->done = true;
    mtx_unlock(&cod->cse->lock);
    wakeup(crp);    
}

-- 
You are receiving this mail because:
You are the assignee for the bug.