aboutsummaryrefslogtreecommitdiffstats
path: root/TODO.md (follow)
Commit message (Collapse)AuthorAgeFilesLines
* global: replace rwlock with mtx if never rlockedJason A. Donenfeld2021-06-051-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* TODO: add note about excessive rw locksJason A. Donenfeld2021-05-031-0/+1
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: handle if_transmit and if_output properlyJason A. Donenfeld2021-04-271-3/+1
| | | | | | | | | | | | | | | | | 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>
* if_wg: do not increment error counter when sc is nullJason A. Donenfeld2021-04-251-0/+5
| | | | | | If sc is null, jumping to increment the counter means crash. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: count on peers always having a remoteJason A. Donenfeld2021-04-241-6/+2
| | | | | | | 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>
* if_wg: remove M_WAITOK, check return codes on initMatt Dunwoodie2021-04-231-1/+0
| | | | | | | | | | | | | | | 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>
* wg_cookie: hash vnet into ratelimiter entryJason A. Donenfeld2021-04-221-3/+0
| | | | | | IPs mean different things per-vnet. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: properly use rn_inithead and rn_detachheadJason A. Donenfeld2021-04-221-2/+0
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* wg_cookie: add cookie_valid boolMatt Dunwoodie2021-04-231-3/+0
| | | | | | | | | | | | | | | | | | | | | | | | 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_cookie: make ratelimiter globalMatt Dunwoodie2021-04-231-2/+1
| | | | Signed-off-by: Matt Dunwoodie <ncon@noconroy.net>
* TODO: more nitsJason A. Donenfeld2021-04-221-2/+12
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* selftests: fixup headersJason A. Donenfeld2021-04-221-1/+0
| | | | | | Also remove the stale entry from the TODO list. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: port allowedips selftest from Linux code and fix bugsJason A. Donenfeld2021-04-221-2/+1
| | | | | | | And then fix broken allowedips implementation for the static unit tests to pass. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* global: use ck for loads/stores, rather than macro mazeJason A. Donenfeld2021-04-201-1/+0
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* TODO: add a few thingsJason A. Donenfeld2021-04-201-0/+4
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* if_wg: replace wg_tag with wg_packetMatt Dunwoodie2021-04-191-5/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | `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>
* if_wg: import latest wg_noise.{c,h}Matt Dunwoodie2021-04-191-2/+0
| | | | | | | | | | | | | | | | | | | | | 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>
* if_wg: remove peer marshalling from get requestJason A. Donenfeld2021-04-131-5/+0
| | | | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
* TODO: initial dumpJason A. Donenfeld2021-03-311-0/+36
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>