[PATCH] wireguard: Mark concurrent accesses to reused indices

[PATCH] wireguard: Mark concurrent accesses to reused indices

[Zhu Jun]

Removing a handshake entry from the index table does not wait for RCU
readers that already found it. Reinserting the entry assigns a new index
while those readers may still compare the old entry's index, which KCSAN
reports as a data race.

Use WRITE_ONCE() for index reassignment and READ_ONCE() for index
comparisons in the table walks. Keep the existing RCU lifetime protection
and locked collision check.

OpenAI Codex assisted with analysis and patch preparation.

Fixes: e7096c131e51 ("net: WireGuard secure network tunnel")
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
 drivers/net/wireguard/peerlookup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireguard/peerlookup.c b/drivers/net/wireguard/peerlookup.c
index 99651a86ac95..c8d943711a6a 100644
--- a/drivers/net/wireguard/peerlookup.c
+++ b/drivers/net/wireguard/peerlookup.c
@@ -129,11 +129,11 @@ __le32 wg_index_hashtable_insert(struct index_hashtable *table,
 
 search_unused_slot:
 	/* First we try to find an unused slot, randomly, while unlocked. */
-	entry->index = (__force __le32)get_random_u32();
+	WRITE_ONCE(entry->index, (__force __le32)get_random_u32());
 	hlist_for_each_entry_rcu_bh(existing_entry,
 				    index_bucket(table, entry->index),
 				    index_hash) {
-		if (existing_entry->index == entry->index)
+		if (READ_ONCE(existing_entry->index) == entry->index)
 			/* If it's already in use, we continue searching. */
 			goto search_unused_slot;
 	}
@@ -145,7 +145,7 @@ __le32 wg_index_hashtable_insert(struct index_hashtable *table,
 	hlist_for_each_entry_rcu_bh(existing_entry,
 				    index_bucket(table, entry->index),
 				    index_hash) {
-		if (existing_entry->index == entry->index) {
+		if (READ_ONCE(existing_entry->index) == entry->index) {
 			spin_unlock_bh(&table->lock);
 			/* If it was stolen, we start over. */
 			goto search_unused_slot;
@@ -208,7 +208,7 @@ wg_index_hashtable_lookup(struct index_hashtable *table,
 	rcu_read_lock_bh();
 	hlist_for_each_entry_rcu_bh(iter_entry, index_bucket(table, index),
 				    index_hash) {
-		if (iter_entry->index == index) {
+		if (READ_ONCE(iter_entry->index) == index) {
 			if (likely(iter_entry->type & type_mask))
 				entry = iter_entry;
 			break;
-- 
2.51.0

Re: [PATCH] wireguard: Mark concurrent accesses to reused indices

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] wireguard: Mark concurrent accesses to reused indices

[Zhu Jun]

Hi,

The access pair addressed by this patch was observed at runtime by
syzbot/KCSAN. The report is available here:
https://syzbot.org/bug?extid=54fe9e7dbf4babbaeea3

It records a 4-byte write in wg_index_hashtable_insert()
(drivers/net/wireguard/peerlookup.c:132) racing with a 4-byte read in
wg_index_hashtable_lookup() (drivers/net/wireguard/peerlookup.c:211).

I have not reproduced the race locally or verified at runtime that this
patch eliminates the report. My local validation was limited to W=1
builds of the affected object on x86-64 and i386, including an x86-64
KCSAN_STRICT build. I have not demonstrated a functional failure such as
a crash or incorrect packet processing beyond the reported data race.

If another revision is needed for other reasons, I will include this
report and the validation limitations in the commit message.

Thanks,
Zhu Jun