Page MenuHomeFreeBSD

if_wg: avoid sleeping under the net epoch
ClosedPublic

Authored by kevans on Mar 8 2021, 6:19 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 27, 11:46 PM
Unknown Object (File)
Thu, Oct 17, 10:08 PM
Unknown Object (File)
Thu, Oct 17, 10:08 PM
Unknown Object (File)
Thu, Oct 17, 10:07 PM
Unknown Object (File)
Thu, Oct 17, 10:07 PM
Unknown Object (File)
Thu, Oct 17, 9:57 PM
Unknown Object (File)
Wed, Oct 16, 12:01 PM
Unknown Object (File)
Oct 3 2024, 12:59 PM
Subscribers

Details

Summary

No sleeping allowed here, so avoid it. Collect the subset of data we
want inside of the epoch, as we'll need extra allocations when we add
items to the nvlist.

Diff Detail

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

Event Timeline

This revision is now accepted and ready to land.Mar 8 2021, 8:04 AM
sys/dev/if_wg/module/module.c
74

Why NOISE_KEY_SIZE instead of WG_KEY_SIZE?

404–407

Perhaps assert that we're in the net epoch here?

434–436

How do you guarantee that the length of the list doesn't increase between the first loop and this one?

sys/dev/if_wg/module/module.c
74

Stole from the r_public member of struct noise_remote that we're copying into here; the distinction doesn't seem to make much sense to me, as we do use them interchangeably elsewhere (see: noise_local_keys() definition then usage over in wg_cloneattach()).

I'll switch it over to WG_KEY_SIZE, though.

434–436

Whoops, brain-o; will fix to stop at exp->aip_count and update it after.

kevans marked 3 inline comments as done.

Address feedback from @markj

This revision now requires review to proceed.Mar 8 2021, 6:50 PM
sys/dev/if_wg/module/module.c
499

Is it intentional that an error part-way though will result in this function returning success with a truncated list?

533

nvl is leaked in this error path, not a new bug though.

I think this is ok, my last questions are around preexisting behaviour.

This revision is now accepted and ready to land.Mar 8 2021, 9:06 PM
This revision was automatically updated to reflect the committed changes.
kevans marked 2 inline comments as done.