Page MenuHomeFreeBSD

ipsec: serialize SPI allocation
ClosedPublic

Authored by mjg on Nov 3 2021, 4:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 14 2022, 3:03 AM
Unknown Object (File)
Dec 13 2022, 5:49 AM
Unknown Object (File)
Dec 13 2022, 5:49 AM
Unknown Object (File)
Dec 13 2022, 5:48 AM
Unknown Object (File)
Dec 11 2022, 6:51 AM
Subscribers

Details

Summary

two commits:

ipsec: add a lock encompassing SPI allocation

SPIs get allocated and inserted in separate steps. Prior to the change
there was nothing preventing 2 differnet threads from ending up with the
same one.

ipsec: fix edge case detection in key_do_getnewspi

The 'count' variable would end up being -1 post loop, while the
following condition would check for 0 instead.
Test Plan

kyua test sys/netipsec

internal testing at netgate

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mjg requested review of this revision.Nov 3 2021, 4:25 PM
mjg edited the test plan for this revision. (Show Details)
mjg added inline comments.
sys/netipsec/key.c
219

i tried an mtx lock but it ran into problems with apparent malloc called with M_WAITOK down the line

sys/netipsec/key.c
5638

missing SPI_ALLOC_UNLOCK();

6030

It seems the locking here needed only to satisfy KASSERT in key_do_getnewspi(). So you can unlock here and remove SPI_ALLOC_UNLOCK() in each following condition.

6242

the same, only one SPI_ALLOCK_UNLOCK() here needed.

mjg added inline comments.
sys/netipsec/key.c
6241

key_getsavbyspi does not need the lock per se but I would argue it is more future-proof to keep it

This revision is now accepted and ready to land.Nov 3 2021, 6:10 PM