fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()
ClosedPublic
Actions

Authored by jason_eggnet.com on Mar 8 2018, 4:01 PM.

Details

Reviewers

ae
rwatson
sbruno

Group Reviewers

transport
network

Commits

rS335962: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()

Summary

IPSEC_PCBCTL() functions, which include tcp_ipsec_pcbctl(),
ipsec4_pcbctl(), and ipsec6_pcbctl(), should all have matching locking
semantics.

ipsec4_pcbctl() and ipsec6_pcbctl() expect the inp to be unlocked on
entry and exit and appear to be correctly implemented as such. But
tcp_ipsec_pcbctl() had other semantics. This patch fixes the semantics
for tcp_ipsec_pcbctl().

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jason_eggnet.com created this revision.Mar 8 2018, 4:01 PM

Herald added subscribers: ae, vangyzen. · View Herald TranscriptMar 8 2018, 4:01 PM

Harbormaster completed remote builds in B15436: Diff 40076.Mar 8 2018, 4:01 PM

swills added a subscriber: swills.Mar 8 2018, 4:39 PM

jason_eggnet.com added reviewers: transport, network, ae, rwatson.Mar 8 2018, 4:44 PM

sbruno accepted this revision.Mar 8 2018, 5:13 PM

This revision is now accepted and ready to land.Mar 8 2018, 5:13 PM

Why you need INP_WLOCK() for getsockopt()? Also ipsec_get_pcbpolicy() requieres INP_WLOCK(), and IPv4 version uses WLOCK, but IPv6 version uses RLOCK.

This revision now requires changes to proceed.Mar 9 2018, 6:59 AM

Also, both setsockopt() have the check

if (!error)
         INP_WUNLOCK(in6p);

but both getsockopt() have not it.

fix IPv6 IPSEC_PCBCTL locking to be consistent with IPv4

Harbormaster completed remote builds in B15449: Diff 40103.Mar 9 2018, 3:48 PM

In D14623#307351, @ae wrote:

Why you need INP_WLOCK() for getsockopt()? Also ipsec_get_pcbpolicy() requieres INP_WLOCK(), and IPv4 version uses WLOCK, but IPv6 version uses RLOCK.

I have just realized that the pcbctl method TCPMD5_PCBCTL() uses is different from the pbctl method that IPSEC_PCBCTL() uses.

The former is in struct tcpmd5_methods and the latter is in struct ipsec_methods. The API does not have to be the same between them.

I will update shortly :)

fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()

IPSEC_PCBCTL() functions, which include tcp_ipsec_pcbctl(),
ipsec4_pcbctl(), and ipsec6_pcbctl(), should all have matching locking
semantics.

Harbormaster completed remote builds in B15466: Diff 40125.Mar 10 2018, 1:06 AM

jason_eggnet.com retitled this revision from Fix locking for IPSEC_PCBCTL and related functions. to fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl().Mar 10 2018, 1:07 AM

jason_eggnet.com edited the summary of this revision. (Show Details)

sbruno accepted this revision.Mar 22 2018, 11:37 PM

I believe I have addressed all concerns, primarily by more fully understanding the functions that were interrelated and realizing that there wasn't anywhere near as much to fix in the first place. The earlier revisions of this patch were far reaching, but now the fixes are just internal to tcp_ipsec_pcbctl()

This revision was not accepted when it landed; it landed in state Needs Review.Jul 4 2018, 5:10 PM

Closed by commit rS335962: fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl() (authored by sbruno). · Explain Why

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptJul 4 2018, 5:10 PM