| Age | Commit message (Collapse) | Author |
|
|
|
Reported-by: syzbot+8c519140cac567be1ee1@syzkaller.appspotmail.com
|
|
This gives us the opportunity to detect usage mistakes like
use-after-free.
Exception: Continue to silently ignore sid=0.
|
|
|
|
Same with crypto_kdispatch.
|
|
No other errors are allowed -- other errors must be transmitted by
crypto_done. All drivers in tree (sun8i_crypto, glxsb, via_padlock,
mvcesa, mvxpsec, hifn, qat, ubsec, cryptosoft) have been audited for
this.
|
|
I'm pretty sure this never worked reliably based on code inspection,
and it's unlikely to have ever been tested because it only applies
when unregistering a driver -- but we have no crypto drivers for
removable devices, so it would only apply if we went out of our way
to trigger detach with drvctl.
Instead, just make the operation fail with ENODEV, and remove all the
callback logic to resubmit the request on EAGAIN. (Maybe this should
be ENXIO, but crypto_kdispatch already does ENODEV.)
|
|
Same with crypto_kdispatch.
Convert some dead branches downstream to assertions too.
|
|
|
|
No callers use the return value. It is not sensible to allow this to
fail.
|
|
No functional change intended: all drivers already return zero
unconditionally.
|
|
It can only fail if we pass it an invalid sid, which the logic to
maintain the user sessions should not do. So kassert error=0 here.
|
|
|
|
- crypto_getreq ensures crp_desc is nonnull.
- Caller is responsible for setting crp_buf.
|
|
This way we avoid passing 0 to crypto_getreq -- makes it easier to
reason about everything downstream.
|
|
- For crypto_getreq this makes downstream reasoning easier: on
success, crp_desc is guaranteed to be nonnull.
- For crypto_kgetreq, this was already assumed, just silently
ignored and not checked by anything.
|
|
Nothing uses the return values inside opencrypto, so let's stop
making users return them.
|
|
Nothing uses it any more.
|
|
This is extremely unlikely because I don't think we have any drivers
for removable crypto decelerators^Waccelerators...but if we were to
sprout one, and someone ran crypto_dispatch concurrently with
crypto_unregister, cryptodev_cb/mcb would enter with crp->crp_etype =
EAGAIN and with CRYPTO_F_DONE set in crp->crp_flags. In this case,
cryptodev_cb/mcb would issue crypto_dispatch but -- since nothing
clears CRYPTO_F_DONE -- it would _also_ consider the request done and
notify the ioctl thread of that.
With this change, we return early if crypto_dispatch succeeds. No
need to consult CRYPTO_F_DONE: if the callback is invoked it's done,
and if we try to redispatch it on EAGAIN but crypto_dispatch fails,
it's done. (Soon we'll get rid of the possibility of crypto_dispatch
failing synchronously, but not just yet.)
XXX This path could really use some testing!
|
|
This is no longer necessary because crypto_freesession no longer
calls into the driver for session ids that were never allocated in
the first place.
|
|
Previously, crypto_newsession could sometimes return 0 as the
driver-specific part of the session id, and 0 as the hid, for sid=0.
But netipsec assumes that it is always safe to free sid=0 from
zero-initialized memory even if crypto_newsession has never
succeeded. So it was up to every driver in tree to gracefully handle
sid=0, if it happened to get assigned hid=0. And, as long as the
freesession callback was expected to just return an error code when
given a bogus session id, that worked out fine...because nothing ever
used the error code.
That was a terrible fragile system that should never have been
invented. Instead, let's just ensure that valid session ids are
nonzero, and make crypto_freesession with sid=0 be a no-op.
|
|
This deliberately ignores the error code returned by crypto_dispatch,
but that error code is fundamentally incoherent and the issue will be
mooted by subsequent changes to make it return void and always pass
the error through the callback, as well as subsequent changes to rip
out the EAGAIN logic anyway.
|
|
The condvar may be destroyed by the time we got here, and nothing
waits on it anyway -- instead the caller is expected to select/poll
for completion in userland.
The bug was already here, but the recent change to eliminate
CRYPTO_F_CBIMM made it happen more often by causing the callback to
_always_ be run asynchronously instead of sometimes being run
synchronously.
|
|
These should only ever have been potentially called from hard
interrupt context by CRYPTO_F_CBIMM callbacks (CBIMM = call back
immediately). CRYPTO_F_CBIMM is no more, so there is no more need to
allow this case of call from hard interrupt context.
|
|
CRYPTO_F_USER is no longer needed. It was introduced in 2008 by
darran@ in crypto.c 1.30, cryptodev.c 1.45 in an attempt to avoid
double-free between the issuing thread and asynchronous callback.
But the `fix' didn't work. In 2017, knakahara@ fixed it properly in
cryptodev.c 1.87 by distinguishing `the crypto operation has
completed' (CRYPTO_F_DONE) from `the callback is done touching the
crp object' (CRYPTO_F_DQRETQ, now renamed to CRYPTODEV_F_RET).
CRYPTO_F_CBIMM formerly served to invoke the callback synchronously
from the driver's interrupt completion routine, to reduce contention
on what was once a single cryptoret thread. Now, there is a per-CPU
queue and softint for much cheaper processing, so there is less
motivation for this in the first place. So let's remove the
complicated logic. This means the callbacks never run in hard
interrupt context, which means we don't need to worry about recursion
into crypto_dispatch in hard interrupt context.
|
|
No functional change intended.
|
|
No functional change intended -- this only removes an unnecessary
lock/unlock cycle in the error case.
|
|
|
|
These are used (pool_get/put) only from thread context, never from
interrupt or even soft interrupt context.
|
|
This had been done in 1.30 when the locking was different. No need
any more. No functional change intended.
|
|
Just declare the right types to begin with. No functional change
intended.
|
|
interfaces, make sure that initialization and destruction
follow the proper sequence. This is triggered by the recent
changes to the devsw stuff; per riastradh@ the required call
sequence is:
devsw_attach()
config_init_component() or config_cf*_attach()
...
config_fini_component() or config_cf*_detach()
devsw_detach()
While here, add a few missing calls to some of the detach
routines.
Testing of these changes has been limited to:
1. compile without build break
2. no related test failures from atf
3. modload/modunload work as well as
before.
No functional device testing done, since I don't have any
of these devices. Let me know of any damage I might cause
here!
XXX Some of the modules affected by this commit are already
XXX broken; see kern/56772. This commit does not break
any additional modules (as far as I know).
|
|
This hasn't worked since it was written in 2009; if anyone cared
surely they would have fixed it by now!
(Fixing this properly -- and putting a more reasonable upper bound
than the maximum that size_t arithmetic allows -- left as an exercise
or the reader.)
Reported-by: syzbot+798d4a16bc15ae88526e@syzkaller.appspotmail.com
|
|
while here fix few typos in comments.
|
|
|
|
|
|
|
|
|
|
|
|
Should address this bracket report that has my name all over it:
https://mail-index.netbsd.org/current-users/2020/07/04/msg039059.html
|
|
Update netipsec dependency.
|
|
While here, apply various rijndael->aes renames, reduce the size
of aesxcbc_ctx by 480 bytes, and convert some malloc->kmem.
Leave in the symbol enc_xform_rijndael128 for now, though, so this
doesn't break any kernel ABI.
|
|
|
|
depend on it. Dependency is explicitly declared in files.foo if a
component requires it.
|
|
and pool_prime() (and their pool_cache_* counterparts):
- the pool_set*wat() APIs are supposed to specify thresholds for the count of
free items in the pool before pool pages are automatically allocated or freed
during pool_get() / pool_put(), whereas pool_sethardlimit() and pool_prime()
are supposed to specify minimum and maximum numbers of total items
in the pool (both free and allocated). these were somewhat conflated
in the existing code, so separate them as they were intended.
- change pool_prime() to take an absolute number of items to preallocate
rather than an increment over whatever was done before, and wait for
any memory allocations to succeed. since pool_prime() can no longer fail
after this, change its return value to void and adjust all callers.
- pool_setlowat() is documented as not immediately attempting to allocate
any memory, but it was changed some time ago to immediately try to allocate
up to the lowat level, so just fix the manpage to describe the current
behaviour.
- add a pool_cache_prime() to complete the API set.
|
|
For some reason, the crypto module fails to link, and this results in
opencrypto sysctl failures.
Should resolve PR kern/55154
|
|
automate installation of sysctl nodes.
Note that there are still a number of device and pseudo-device modules
that create entries tied to individual device units, rather than to the
module itself. These are not changed.
|
|
|
|
Can't sleep for allocation in percpu_foreach.
|
|
|