netinet6: remove INP_DROPPED checks from setsockopt(2)
ClosedPublic
Actions

Authored by glebius on Mon, Mar 30, 11:00 PM.

Details

Reviewers

markj
pouria

Group Reviewers

transport
network

Commits

rGce283e115b02: netinet6: remove INP_DROPPED checks from setsockopt(2)

Summary

The INP_DROPPED is going to become an internal flag for inpcb. As of now
it means a TCP pcb that is in TCPS_CLOSED. There is nothing wrong with
calling setsockopt(2) on such socket, although has no practical use.

This deletes a piece of code from 56713d16a06c5 / D16201. There is no
description of the panic fixed, but I will speculate that the panic was
about in6p->in6p_outputopts being NULL as the inpcb already went through
in_pcbfree_deferred(). This also can be related to compressed TIME-WAIT,
that is also gone now.

With current locking this shouldn't be possible. An inpcb goes through
in_pcbfree() only with pr_detach method, which is called from sofree(),
and the latter is called on losing the very last socket reference. So, at
the point when in_pcbfree() is called, the socket has lost its file
descriptor reference and there can not be any running setsockopt() on it.

Leave the call to ip6_pcbopt() still embraced with INP_WLOCK(), since we
are modifying inpcb contents.

NB: the IPv6 setsockopt(2) definitely has room for improvement. Several
memory allocations should be moved out of lock and made M_WAITOK.
Covering large piece of setsockopt(2) code with epoch(9) just because
ip6_setpktopts() calls ifnet_byindex() isn't correct either.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 71835
Build 68718: arc lint + arc unit

Event Timeline

glebius created this revision.Mon, Mar 30, 11:00 PM

Herald added a reviewer: pouria. · View Herald TranscriptMon, Mar 30, 11:00 PM

Herald added subscribers: melifaro, ae, imp. · View Herald Transcript

glebius requested review of this revision.Mon, Mar 30, 11:00 PM

Harbormaster completed remote builds in B71835: Diff 174583.Mon, Mar 30, 11:00 PM

glebius added a parent revision: D55968: inpcb: make in_pcbdisconnect() acquire the hash lock internally.Mon, Mar 30, 11:00 PM

markj accepted this revision.Tue, Mar 31, 2:27 AM

markj added inline comments.

sys/netinet6/ip6_output.c
2417	GET_PKTOPT_VAR is used from ip6_pcbgetopt(), which does not assume that a socket reference is present. If in the future some new caller appears, outside of ip6_ctloutput(), then we will still need to make sure that the PCB has not been detached from the protocol, otherwise there will be a leak or panic (if the in6p_outputopts pointer has been poisoned by in_pcbfree()). It would be nice to have some guarantee that the socket has not yet been detached.