Page MenuHomeFreeBSD

if_ovpn: Destroy cloned interfaces via a prison removal callback
ClosedPublic

Authored by markj on Jul 25 2025, 2:22 PM.
Tags
None
Referenced Files
F132455641: D51526.id159086.diff
Fri, Oct 17, 2:15 AM
Unknown Object (File)
Sat, Oct 11, 7:29 AM
Unknown Object (File)
Sat, Oct 11, 7:29 AM
Unknown Object (File)
Sat, Oct 11, 7:29 AM
Unknown Object (File)
Sat, Oct 11, 7:29 AM
Unknown Object (File)
Sat, Oct 11, 7:28 AM
Unknown Object (File)
Sat, Oct 11, 12:10 AM
Unknown Object (File)
Tue, Oct 7, 9:40 AM

Details

Summary

A if_ovpn interface carries a reference to a socket, which has a
credential reference, which holds a reference on the containing prison
and prevents SYSUNINITs from being invoked. So, register a
PR_METHOD_REMOVE callback and destroy the cloner from there instead,
since that mechanism doesn't require the prison refcount to drop to zero
first.

This fixes a bug where jails get left stuck in the DYING state after
running if_ovpn regression tests.

Diff Detail

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

Event Timeline

markj requested review of this revision.Jul 25 2025, 2:22 PM

Should we unconditionally detach the V_ovpn_cloner cloner in the non-VIMAGE case? Or under MOD_UNLOAD?
If I'm reading this right we wouldn't do so now, so we could end up unloading if_ovpn.ko and being left with the cloner functions pointing to now uninitialised memory.

Be sure to detach the root VNET's cloner during module unload.

In D51526#1178362, @kp wrote:

Should we unconditionally detach the V_ovpn_cloner cloner in the non-VIMAGE case? Or under MOD_UNLOAD?
If I'm reading this right we wouldn't do so now, so we could end up unloading if_ovpn.ko and being left with the cloner functions pointing to now uninitialised memory.

Yes, you're completely right. I think it has to happen under MOD_UNLOAD.

sys/net/if_ovpn.c
2799

I think I'd feel better if we did V_ovpn_cloner = NULL here, because I can't quite convince myself that we're not going to double-detach the host vnet's cloner on module unload.

markj marked an inline comment as done.

Defend against double detaches of the cloner.

sys/net/if_ovpn.c
2796
/usr/src/sys/net/if_ovpn.c:2796:23: error: incompatible pointer types passing 'struct prison *' to parameter of type 'struct ucred *' [-Werror,-Wincompatible-pointer-types]
 2796 |         if (prison_owns_vnet(pr)) {
      |                              ^~
/usr/src/sys/sys/jail.h:438:37: note: passing argument to parameter here
  438 | bool prison_owns_vnet(struct ucred *);
      |                                     ^
1 error generated.
sys/net/if_ovpn.c
2796

Sorry, yes, this depends on D51524. I forgot to create a link in the patch stack.

Ah, that makes sense. I was pretty surprised that your patch didn't build.

This revision is now accepted and ready to land.Jul 28 2025, 3:06 PM