| Message ID | 20200909115815.522168-1-Jason@zx2c4.com |
|---|---|
| Headers | show |
| Series | wireguard fixes for 5.9-rc5 | expand |
From: "Jason A. Donenfeld" <Jason@zx2c4.com> Date: Wed, 9 Sep 2020 13:58:13 +0200 > Yesterday, Eric reported a race condition found by syzbot. This series > contains two commits, one that fixes the direct issue, and another that > addresses the more general issue, as a defense in depth. > > 1) The basic problem syzbot unearthed was that one particular mutation > of handshake->entry was not protected by the handshake mutex like the > other cases, so this patch basically just reorders a line to make > sure the mutex is actually taken at the right point. Most of the work > here went into making sure the race was fully understood and making a > reproducer (which syzbot was unable to do itself, due to the rarity > of the race). > > 2) Eric's initial suggestion for fixing this was taking a spinlock > around the hash table replace function where the null ptr deref was > happening. This doesn't address the main problem in the most precise > possible way like (1) does, but it is a good suggestion for > defense-in-depth, in case related issues come up in the future, and > basically costs nothing from a performance perspective. I thought it > aided in implementing a good general rule: all mutators of that hash > table take the table lock. So that's part of this series as a > companion. > > Both of these contain Fixes: tags and are good candidates for stable. Series applied and queued up for -stable, thanks.
Hi Dave, Yesterday, Eric reported a race condition found by syzbot. This series contains two commits, one that fixes the direct issue, and another that addresses the more general issue, as a defense in depth. 1) The basic problem syzbot unearthed was that one particular mutation of handshake->entry was not protected by the handshake mutex like the other cases, so this patch basically just reorders a line to make sure the mutex is actually taken at the right point. Most of the work here went into making sure the race was fully understood and making a reproducer (which syzbot was unable to do itself, due to the rarity of the race). 2) Eric's initial suggestion for fixing this was taking a spinlock around the hash table replace function where the null ptr deref was happening. This doesn't address the main problem in the most precise possible way like (1) does, but it is a good suggestion for defense-in-depth, in case related issues come up in the future, and basically costs nothing from a performance perspective. I thought it aided in implementing a good general rule: all mutators of that hash table take the table lock. So that's part of this series as a companion. Both of these contain Fixes: tags and are good candidates for stable. Jason A. Donenfeld (2): wireguard: noise: take lock when removing handshake entry from table wireguard: peerlookup: take lock before checking hash in replace operation drivers/net/wireguard/noise.c | 5 +---- drivers/net/wireguard/peerlookup.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 7 deletions(-) Cc: Eric Dumazet <edumazet@google.com> -- 2.28.0