Page MenuHomeFreeBSD

tun: VIMAGE fix for if_tun cloner
ClosedPublic

Authored by kp on Feb 19 2019, 6:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 20, 2:04 PM
Unknown Object (File)
Sat, Mar 2, 3:47 AM
Unknown Object (File)
Jan 5 2024, 3:15 AM
Unknown Object (File)
Jan 3 2024, 3:04 AM
Unknown Object (File)
Dec 20 2023, 5:31 AM
Unknown Object (File)
Dec 19 2023, 9:19 PM
Unknown Object (File)
Dec 15 2023, 2:11 AM
Unknown Object (File)
Nov 26 2023, 8:39 PM

Details

Summary
tun: VIMAGE fix for if_tun cloner

The if_tun cloner is not virtualised, but if_clone_attach() does use a
virtualised list of cloners.
The result is that we can't find the if_tun cloner when we try to remove
a renamed tun interface. Virtualise the cloner, and move the final
cleanup into a sysuninit so that we're sure this happens after all of
the vnet_sysuninits

Note that we need unit numbers to be system-unique (rather than unique
per vnet, as is done by if_clone_simple()). The unit number is used to
create the corresponding /dev/tunX device node, and this node must match
with the interface.
Switch to if_clone_advanced() so that we have control over the unit
numbers.

Reproduction scenario:
        jail -c -n foo persist vnet
        jexec test ifconfig tun create
        jexec test ifconfig tun0 name wg0
        jexec test ifconfig wg0 destroy

PR:     235704

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22766
Build 21858: arc lint + arc unit

Event Timeline

Note that the character device subsystem is not virtualized, so tunX must be system uniq.

Thanks. The original reporter of the bug discovered that too.

I'll see what I can do about that.

Ensure that unit numbers are system-unique.

Remove stray debug lines

sys/net/if_tun.c
182

Technically you should check there is a digit after tun?

if (name[0] == 't' && name[1] == 'u' && name[2] == 'n' && isdigit(name[3]))

sys/net/if_tun.c
182

That'd break ifconfig tun create.

sys/net/if_tun.c
182

So check for (name[3] == 0 || isdigit(name[3]))

strncmp() will match anything that starts with tun.

sys/net/if_tun.c
182

That is a good idea. I'll update the patch.

Fix tun_clone_match() to not match on e.g. tunnel.

hrs added inline comments.
sys/net/if_tun.c
137

This should be VNET_DEFINE_STATIC() instead of VNET_DEFINE().

206

I think "%s%d", tunname, unit is better than a string literal "tun" here.

Fix issues pointed out by hrs.

kp marked 4 inline comments as done.Feb 27 2019, 9:13 PM

If there are no objections I'm going to commit this soon.

This revision is now accepted and ready to land.Mar 4 2019, 6:43 PM

I hate manually virtualised cloners but people changed if_clone.c logic under the virtualised cloner infrastructure and I gave up on this years ago. Sorry I am not really helpful here currently. Also I think it needs more words describing the problem and part of the solution as I cannot imagine this only being a problem of tun(4) left but probably another few others?

sys/net/if_tun.c
183

Probably '\0'

This revision now requires review to proceed.Mar 5 2019, 9:23 AM

Thanks for updating the description; The repro scenario really should be a test case. Accept for the general idea, not for having checked the code in detail.

This revision is now accepted and ready to land.Mar 5 2019, 9:34 AM
This revision was automatically updated to reflect the committed changes.