if_tuntap: avoid a deadlock in SIOCIFDESTROY

[kevans]

Summary

tp->tun_pid is only updated on a best-effort basis, but it's still an OK
heuristic for this. If the thread that called SIOCIFDESTROY had the
tunnel open at the time, we'd immediately block on the tun condvar for
a release that will never come. We'll only return a spurious EBUSY if
the owner hasn't been updated, which would have otherwise blocked
because the tunnel *is* open; this seems like an acceptable failure
mode.

PR: 242841

[markj]

Would it be possible to make the sleep in tun_destroy() interruptible?

sys/net/if_tuntap.c

663

A comment seems warranted here.

[kevans]

In D39722#904222, @markj wrote:

Would it be possible to make the sleep in tun_destroy() interruptible?

Hmm... yeah, we could probably do that. We'd need to push removal from the tun_list into tun_clone_destroy and it may be a little wonky due to locking differences between the two callers, but not that bad.

[melifaro]

Thank you for rootcausing the problem!
This approach worries me a bit - what if we have a third-party buggy library and the core app that tries to fix it by manually deleting the interfaces?
I’d really prefer SIOCIFDESTROY to always do what it’s asked for regardless of the fd open/close status

[kevans]

In D39722#904270, @melifaro wrote:

Thank you for rootcausing the problem!
This approach worries me a bit - what if we have a third-party buggy library and the core app that tries to fix it by manually deleting the interfaces?
I’d really prefer SIOCIFDESTROY to always do what it’s asked for regardless of the fd open/close status

right, if we "just" make the sleep interruptible then this doesn't need to exist anymore; right now, the situation as described in the PR effectively hangs the entire process with not much recourse available to save the process from just being stuck there evermore.

[kevans]

Ah, I guess you're saying you want it to just delete the interface immediately... feel free, but I don't really have the time or inclination to dig through and sus out every part where this may go wrong for whatever has the fd open.

[melifaro]

In D39722#904279, @kevans wrote:

Ah, I guess you're saying you want it to just delete the interface immediately... feel free, but I don't really have the

Not immediately, but “eventually”.
I mean, it is a bit weird one can destroy the openef interface from any other process excluding its own.

time or inclination to dig through and sus out every part where this may go wrong for whatever has the fd open.

[kevans]

In D39722#904299, @melifaro wrote:

In D39722#904279, @kevans wrote:

Ah, I guess you're saying you want it to just delete the interface immediately... feel free, but I don't really have the

Not immediately, but “eventually”.
I mean, it is a bit weird one can destroy the openef interface from any other process excluding its own.

To be fair, they actually can't -- those ones will just hang until the last close.

[melifaro]

In D39722#904300, @kevans wrote:

In D39722#904299, @melifaro wrote:

In D39722#904279, @kevans wrote:

Ah, I guess you're saying you want it to just delete the interface immediately... feel free, but I don't really have the

Not immediately, but “eventually”.
I mean, it is a bit weird one can destroy the openef interface from any other process excluding its own.

To be fair, they actually can't -- those ones will just hang until the last close.

Whoops. Indeed, you're right. I may have a false memory that it was possible some time ago. Apparently it's not.

Sorry for confusion, no objection from myself then.

[kevans]

I think I'll drop this in favor of D39740 -- I'm not sure we need to do this to workaround an application bug if the bug is made generally harmless.