There were multiple places where a rwlock was used despite never
rlocking, so just change these into mtxs. This was done with the aid of
Coccinelle's spatch, using this input:
#spatch -j 4 --recursive-includes --include-headers-for-types --include-headers --in-place --macro-file <seebelow.h>
virtual after_start
@initialize:ocaml@
@@
let has_write_table = Hashtbl.create 101
let has_read_table = Hashtbl.create 101
let ok i m =
let entry = (i,m) in
Hashtbl.mem has_write_table entry && not(Hashtbl.mem has_read_table entry)
@hasw depends on !after_start@
identifier i,m;
struct i x;
@@
(
rw_wlock(&x.m)
|
rw_wunlock(&x.m)
)
@script:ocaml@
i << hasw.i;
m << hasw.m;
@@
Hashtbl.replace has_write_table (i,m) ()
@hasr depends on !after_start@
identifier i,m;
struct i x;
@@
(
rw_rlock(&x.m)
|
rw_runlock(&x.m)
)
@script:ocaml@
i << hasr.i;
m << hasr.m;
@@
Hashtbl.replace has_read_table (i,m) ()
@finalize:ocaml depends on !after_start@
wt << merge.has_write_table;
rt << merge.has_read_table;
@@
let redo ts dst =
List.iter (Hashtbl.iter (fun k _ -> Hashtbl.add dst k ())) ts in
redo wt has_write_table;
redo rt has_read_table;
let it = new iteration() in
it#add_virtual_rule After_start;
it#register()
(* ----------------------------------------------------------- *)
@depends on after_start@
identifier i;
identifier m : script:ocaml(i) { ok i m };
@@
struct i {
...
- struct rwlock m;
+ struct mtx m;
...
}
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rw_wlock
+ mtx_lock
(&x.m)
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rw_wunlock
+ mtx_unlock
(&x.m)
@depends on after_start disable fld_to_ptr@
identifier m;
expression e;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rw_init(&x.m, e);
+ mtx_init(&x.m, e, NULL, MTX_DEF);
@depends on after_start disable fld_to_ptr@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i x;
@@
- rw_destroy
+ mtx_destroy
(&x.m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rw_wlock
+ mtx_lock
(&x->m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rw_wunlock
+ mtx_unlock
(&x->m)
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
expression e;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rw_init(&x->m, e);
+ mtx_init(&x->m, e, NULL, MTX_DEF);
@depends on after_start disable fld_to_ptr, ptr_to_array@
identifier m;
identifier i : script:ocaml(m) { ok i m };
struct i *x;
@@
- rw_destroy
+ mtx_destroy
(&x->m)
A few macros needed to be provided manually for the parser to work:
#define LIST_HEAD(x,y) int
#define TAILQ_HEAD(x,y) int
#define STAILQ_HEAD(x,y) int
#define CK_LIST_HEAD(x,y) int
#define CK_LIST_ENTRY(x) int
#define LIST_ENTRY(x) int
#define TAILQ_ENTRY(x) int
#define STAILQ_ENTRY(x) int
Co-authored-by: Julia Lawall <julia.lawall@inria.fr>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
The netmap code goes directly to if_transmit, which means it'll bypass
if_output, in which case, there's no packet allocated. Also, we're
relying on if_output's sockaddr structure to be legit, but who knows
what types of userspace hijynxes can forge this. Rather than relying on
that kind of black magic, determine the AF from the actual packet
contents. But still insist that it agrees with the sockaddr.
The extraction of the type from AF_UNSPEC follows the same pattern as
if_gif and if_gre.
We also use this as an opportunity to send icmp error messages in the
right place.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
We do a pretty nasty hack in the allowedips selftest to avoid having to
allocate more memory. Seems to work.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Here we remove all M_WAITOK checks, because we don't want to hang while
trying to allocate memory. It is better to return an error so the user
can try again later.
We also make sure to check all the return codes in peer and interface
allocation. The structure of those functions is:
1) Allocate all memory
2) Initialise fields in order of the struct
3) Cleanup gotos
Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
Primarily this commit adds a cookie_valid state, to prevent a recently
booted machine from sending a mac2. We also do a little bit of reworking
on locking and a fixup for int to bool.
There is one slight difference to cookie_valid (latest_cookie.is_valid)
on Linux and that is to set cookie_valid to false when the
cookie_birthdate has expired. The purpose of this is to prevent the
expensive timer check after it has expired.
For the locking, we want to hold a write lock in cookie_maker_mac
because we write to mac1_last, mac1_valid and cookie_valid. This
wouldn't cause too much contention as this is a per peer lock and we
only do so when sending handshake packets. This is different from Linux
as Linux writes all it's variables at the start, then downgrades to a
read lock.
We also match cookie_maker_consume_payload locking to Linux, that is to
read lock while checking mac1_valid and decrypting the cookie then take
a write lock to set the cookie.
Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
`wg_tag` is a source of trouble when it comes to handling mbufs. This is
due to the fact that calls to things like m_prepend may free the mbuf
underneath us, which would be bad if the tag is still queued in the
peer's queue.
`wg_tag` has also been made redundant on other platforms due to size
restrictions (80 bytes on OpenBSD) which means we cannot grow it to the
required size to hold new fields. With wg_packet, this is no longer a
concern.
This patch includes an import of the send/recv paths (from OpenBSD) to
ensure we don't leak an refcounts. This additionally solves two of the
TODOs as well (chop rx padding, don't copy mbuf). The second TODO is
helpful, because we no longer need to allocate mbufs of a specific size
when encrypting, meaning we no longer have an upper bound on the MTU.
(rebase) On second thoughts, that m_defrag is deadly, as it does not
behave the same as m_defrag on OpenBSD. If the packet is large enough,
there will still be multiple clusters, so treating the first mbuf as the
whole buffer may lead to a heap overflow. This is addressed by the
"encrypt mbuf in place" commit, so while is an issue here, it is already
resolved. To say it in caps:
THIS COMMIT INTRODUCES A VULN, FIXED BY: encrypt mbuf in place
There could be some discussion around using p_parallel for the staged
and handshake queues. It isn't as idiomatic as I would like, however the
right structure is there so that is something we could address later.
One other thing to consider is that `wg_peer_send_staged` is likely
being called one packet at a time. Is it worthwhile trying to batch
calls together?
Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
Note: this is a partial diff, introducing temporary bugs that will be
resolved in following commits, detailed below.
This commit brings wg_noise.{c,h} up to date with wireguard-openbsd. The
primary motivator for this large patchset is to allow checking nonces
serial, requiring a reference to the receiving keypair across noise_*
calls. Due to requiring reference counting on the keypairs, we also take
this opportunity to throw away the old locking and bring in EPOCH
(roughly equivalent to SMR on OpenBSD and RCU on Linux).
The changes to if_wg.c are purely to allow it to compile, there are most
certainly refcount leaks present (to be addressed in the following
commits). Readers should review wg_noise.{c,h} in their entirety rather
than the diffs, as there are significant changes. if_wg.c can be
reviewed, but must be contextualised with the following commits
(repace wg_tag with wg_packet, encrypt mbuf in place).
Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
Jason A. Donenfeld