| Commit message (Collapse) | Author | Files | Lines |
|---|
|
This simplifies the deletion process, so we do not require a lookup of
the node before deletion.
|
|
The primary motivator here is to get rid of CONTAINER_OF, which is quite
an ugly macro.
However, any reader should also be aware of the change from d_DISabled
to p_ENabled.
|
|
The lock was not used to protect any data structures, it was purely to
ensure race-free setting of t_disabled. That is, that no other thread
was halfway through any wg_timers_run_* function. With smr_* we can
ensure this is still the case by calling smr_barrier() after setting
t_disabled.
|
|
So the reason timeouts were running in interrupt context was because it
was quicker. Running in process context required a `task` to be added,
which we ended up doing anyway. So we might as well rely on timeout API
to do it for us.
|
|
We can get rid of the pool overhead by using the malloc family of
functions. This does lose us the ability to see directly how much each
allocation is using, but it if we really want that, maybe we add new
malloc types? Either way, not something we need at the moment.
|
|
While the largest change here is to use SMR for wg_noise, this was
motivated by other deficiencies in the module. Primarily, the nonce
operations should be performed in serial (wg_queue_out, wg_deliver_in)
and not parallel (wg_encap, wg_decap). This also brings in a lock-free
encrypt and decrypt path, which is nice.
I suppose other improvements are that local, remote and keypair structs
are opaque, so no more reaching in and fiddling with things.
Unfortunately, these changes make abuse of the API easier (such as
calling noise_keypair_encrypt on a keypair retrieved with
noise_keypair_lookup (instead of noise_keypair_current) as they have
different checks). Additionally, we have to trust that the nonce passed
to noise_keypair_encrypt is non repeating (retrieved with
noise_keypair_nonce_next), and noise_keypair_nonce_check is valid on
received nonces.
One area that could use a little bit more adjustment is the *_free
functions. They are used to call a function once it is safe to free a
parent datastructure (one holding struct noise_{local,remote} *). This
is currently used for lifetimes in the system and allows a consumer of
wg_noise to opaquely manage lifetimes based on the reference counting of
noise, remote and keypair. It is fine for now, but maybe revisit later.
|
|
This function (or of similar nature) is required to safely use a refcnt
and smr_entry together. Such functions exist on other platforms as
kref_get_unless_zero (on Linux) and refcount_acquire_if_gt (on FreeBSD).
The following diagram details the following situation with and without
refcnt_take_if_gt in 3 cases, with the first showing the "invalid" use
of refcnt_take.
Situation:
Thread #1 is removing the global referenc (o).
Thread #2 wants to reference an object (r), using a thread pointer (t).
Case:
1) refcnt_take after Thread #1 has released "o"
2) refcnt_take_if_gt before Thread #1 has released "o"
3) refcnt_take_if_gt after Thread #1 has released "o"
Data:
struct obj {
struct smr_entry smr;
struct refcnt refcnt;
} *o, *r, *t1, *t2;
Thread #1 | Thread #2
---------------------------------+------------------------------------
| r = NULL;
rw_enter_write(&lock); | smr_read_enter();
|
t1 = SMR_PTR_GET_LOCKED(&o); | t2 = SMR_PTR_GET(&o);
SMR_PTR_SET_LOCKED(&o, NULL); |
|
if (refcnt_rele(&t1->refcnt) |
smr_call(&t1->smr, free, t1); |
| if (t2 != NULL) {
| refcnt_take(&t2->refcnt);
| r = t2;
| }
rw_exit_write(&lock); | smr_read_exit();
.....
// called by smr_thread |
free(t1); |
.....
| // use after free
| *r
---------------------------------+------------------------------------
| r = NULL;
rw_enter_write(&lock); | smr_read_enter();
|
t1 = SMR_PTR_GET_LOCKED(&o); | t2 = SMR_PTR_GET(&o);
SMR_PTR_SET_LOCKED(&o, NULL); |
|
if (refcnt_rele(&t1->refcnt) |
smr_call(&t1->smr, free, t1); |
| if (t2 != NULL &&
| refcnt_take_if_gt(&t2->refcnt, 0))
| r = t2;
rw_exit_write(&lock); | smr_read_exit();
.....
// called by smr_thread | // we don't have a valid reference
free(t1); | assert(r == NULL);
---------------------------------+------------------------------------
| r = NULL;
rw_enter_write(&lock); | smr_read_enter();
|
t1 = SMR_PTR_GET_LOCKED(&o); | t2 = SMR_PTR_GET(&o);
SMR_PTR_SET_LOCKED(&o, NULL); |
| if (t2 != NULL &&
| refcnt_take_if_gt(&t2->refcnt, 0))
| r = t2;
if (refcnt_rele(&t1->refcnt) |
smr_call(&t1->smr, free, t1); |
rw_exit_write(&lock); | smr_read_exit();
.....
| // we need to put our reference
| if (refcnt_rele(&t2->refcnt))
| smr_call(&t2->smr, free, t2);
.....
// called by smr_thread |
free(t1); |
---------------------------------+------------------------------------
Currently it uses atomic_add_int_nv to atomically read the refcnt,
but I'm open to suggestions for better ways.
The atomic_cas_uint is used to ensure that refcnt hasn't been modified
since reading `old`.
|
|
The problem with checking peer != NULL is that we already dereference
iter to get i_value. This is what was caught in the index == 0 bug
reported on bugs@. Instead, we should assert that iter != NULL.
This is likely to be removed when adjusting wg_noise.c in the not to
distant future.
|
|
|
|
This will make further work on in place decryption a lot easier.
Additionally, it improves the readability as we can get rid of the
difficult _len variables. The copy in and out of wg_pkt_data is also a
cleaner solution than memcpy nonces and whatnot.
|
|
I'll be the first to admit (but not the first to complain) about the
wg_tag situation. It made it very difficult to manage mbufs (that may be
reallocated with functions such as m_pullup). It was also not clear
where allocation was occuring. This also gets rid of the ring buffers in
wg_softc, which added no performance in this situation. They also used
memory unnecessarily and increased the complexity.
I also used this opportunity to get rid of the confusing t_mbuf/t_done
situation and revert to a more understandable UNCRYPTED/CRYPTED/DEAD
packet state. I don't believe there were any issues with the old style,
but to improve readability is always a welcome addition.
With these changes we can start encrypting packets in place (rather than
copying to a new mbuf), which should increase performance. This also
simplifies length calculations by using m_* functions and reading the
pkthdr length.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
is that IOVA allocations always have a gap in-between which produces a fault
on access. If a transfer to a given allocation runs further than expected
we should be able to see it. We pre-allocate IOVA on bus DMA map creation,
and as long as we don't allocate a PTE descriptor, this comes with no cost.
We have plenty of address space anyway, so adding a page-sized gap does not
hurt at all and can only have positive effects.
Idea from kettenis@
|
|
pointer address. Not allowing this one to be allocated might help find
driver bugs, where the device is programmed with a NULL pointer. We have
plenty of address space anyway, so excluding this single page does not
hurt at all and can only have positive effects.
Idea from kettenis@
|
|
|
|
|
|
Matt Hazinski
|
|
indentation on continuation lines. Prompted by GHPR#185
|
|
|
|
|
|
providers get upset if C_Initialize is not matched with C_Finalize.
From Adithya Baglody via GHPR#234; ok markus
|
|
|
|
were not being dequoted correctly and 2) quoted space in the middle
of a string was being incorrectly split.
A unit test for these cases has already been committed
prompted by and based on GHPR#223 by Eero Häkkinen; ok markus@
|
|
Reported by Preben Guldberg. ok mlarkin@
|
|
for loop. Also in http_finish_connect() if the connect was successful
cleanup the addrinfo struct. It is no longer needed.
Found with deraadt@
|
|
old debugging gunk
ok claudio
|
|
similar that have no isssues. Reported by Michael Paoli. Failing
cases commented out for now.
|
|
Should help for -portable where sometimes the cert.pem is missing.
|
|
ok jsing@ tb@
|
|
ok claudio deraadt
|
|
from Boudewijn Dijkstra
|
|
This was also removed upstream.
OK sthen
|
|
OK sthen
|
|
cipher list if defined. otherwise fallback to libtls default.
ok millert@
|
|
|
|
|
|
|
|
ok drahn
|
|
|
|
from miod
|
|
|
|
|
|
OK claudio@
|
|
|
|
|
Matt Dunwoodie
patrick
tb
djm
dv
claudio
deraadt
otto
inoguchi
eric
florian
jmc
job