[PR #217] [MERGED] fix: initialize header_protection.lock, use down_write in set_key #239

New issue

Closed

opened 2026-10-13 21:07:54 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 21:07:54 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/pull/217
Author: @VaisVaisov
Created: 8/17/2026
Status: ✅ Merged
Merged: 8/27/2026
Merged by: @ygurov

Base: master ← Head: fix/header-protection-locking

📝 Commits (1)

3a7b8c1 fix: initialize header_protection.lock, use down_write in set_key

📊 Changes

2 files changed (+4 additions, -3 deletions)

View changed files

📝 src/device.c (+1 -0)
📝 src/header_protection.c (+3 -3)

📄 Description

Closes #216.

Root cause

Two independent bugs in header_protection.c:

header_protection.lock (an rw_semaphore) is never initialized via init_rwsem() anywhere in the driver. It's only zero-filled via memset(wg, 0, sizeof(*wg)) in wg_setup(). Every other rwsem in the driver (cookie->lock, wg->static_identity.lock, handshake->lock) does get init_rwsem()'d — this one was missed. Using an uninitialized rw_semaphore is undefined behavior.
awg_header_protection_set_key() mutates p->key[] and p->has_protection while holding only down_read(), not down_write(). In fact all three functions in the file (init, set_key, get_key) use down_read() — there's no writer path at all, so there's effectively no mutual exclusion for the mutation.

Together these explain setconf failing with Unable to modify interface: Invalid argument when HeaderProtectionKey is set, even with valid S1-S4 values (well above HEADER_PROTECTION_NONCE_SIZE) — reproduction details are in #216.

Fix

Add init_rwsem(&wg->header_protection.lock) in wg_newlink(), next to the other lock initializations.
Change awg_header_protection_set_key() to use down_write()/up_write().

Testing

Built via DKMS against a patched local checkout, loaded on a real server (Ubuntu 24.04, arm64, kernel 6.17.0-1014-oracle), confirmed setconf with HeaderProtectionKey set succeeds, header protection key shows correctly in awg show, and a full handshake + real traffic works end to end.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/pull/217 **Author:** [@VaisVaisov](https://github.com/VaisVaisov) **Created:** 8/17/2026 **Status:** ✅ Merged **Merged:** 8/27/2026 **Merged by:** [@ygurov](https://github.com/ygurov) **Base:** `master` ← **Head:** `fix/header-protection-locking` --- ### 📝 Commits (1) - [`3a7b8c1`](https://github.com/amnezia-vpn/amneziawg-linux-kernel-module/commit/3a7b8c10b783a8d1b17ee0b61ab2cb2547f687a9) fix: initialize header_protection.lock, use down_write in set_key ### 📊 Changes **2 files changed** (+4 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/device.c` (+1 -0) 📝 `src/header_protection.c` (+3 -3) </details> ### 📄 Description Closes #216. ## Root cause Two independent bugs in `header_protection.c`: 1. `header_protection.lock` (an `rw_semaphore`) is never initialized via `init_rwsem()` anywhere in the driver. It's only zero-filled via `memset(wg, 0, sizeof(*wg))` in `wg_setup()`. Every other rwsem in the driver (`cookie->lock`, `wg->static_identity.lock`, `handshake->lock`) does get `init_rwsem()`'d — this one was missed. Using an uninitialized `rw_semaphore` is undefined behavior. 2. `awg_header_protection_set_key()` mutates `p->key[]` and `p->has_protection` while holding only `down_read()`, not `down_write()`. In fact all three functions in the file (`init`, `set_key`, `get_key`) use `down_read()` — there's no writer path at all, so there's effectively no mutual exclusion for the mutation. Together these explain `setconf` failing with `Unable to modify interface: Invalid argument` when `HeaderProtectionKey` is set, even with valid S1-S4 values (well above `HEADER_PROTECTION_NONCE_SIZE`) — reproduction details are in #216. ## Fix - Add `init_rwsem(&wg->header_protection.lock)` in `wg_newlink()`, next to the other lock initializations. - Change `awg_header_protection_set_key()` to use `down_write()`/`up_write()`. ## Testing Built via DKMS against a patched local checkout, loaded on a real server (Ubuntu 24.04, arm64, kernel 6.17.0-1014-oracle), confirmed `setconf` with `HeaderProtectionKey` set succeeds, `header protection key` shows correctly in `awg show`, and a full handshake + real traffic works end to end. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-13 21:07:54 +00:00