Page MenuHomeFreeBSD

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

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

swills added a subscriber: swills.Mar 8 2018, 4:39 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
ae requested changes to this revision.Mar 9 2018, 6:59 AM

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
ae added a comment.Mar 9 2018, 7:03 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

jason_eggnet.com added a comment.EditedMar 9 2018, 3:59 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.

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().

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
This revision was automatically updated to reflect the committed changes.