netgraph: Fix ng_ether's shutdown handing

[markj]

Summary

When tearing down a VNET, netgraph sends shutdown messages to all of the
nodes before detaching interfaces (SI_SUB_NETGRAPH comes before
SI_SUB_INIT_IF in teardown order). ng_ether nodes handle this by
destroying themselves without detaching from the parent ifnet. Then,
when ifnets go away they detach their ng_ether nodes again, triggering a
use-after-free.

Handle this by modifying ng_ether_shutdown() to detach from the ifnet.
If the shutdown was triggered by an ifnet being destroyed, we will clear
priv->ifp in the ng_ether detach callback, so priv->ifp may be NULL.

Also get rid of the printf in vnet_netgraph_uninit(). It can be
triggered trivially by ng_ether since ng_ether_shutdown() persists the
node unless NG_REALLY_DIE is set.

PR: 233622
Reported by: KASAN

Test Plan

sys/net/if_vlan:basic triggers a KASAN violation when ng_ether is loaded. This commit fixes that.

[donner]

I was confused by your "we will clear priv->ifp in the ng_ether detach callback" sentence.
Clarified for me.

sys/netgraph/ng_ether.c

760

So this is the only effective line in the patch.
It sets the reference from the interface (ipf) to the node (if_l2com) ... to zero.

[markj]

In D27662#618351, @lutz_donnerhacke.de wrote:

I was confused by your "we will clear priv->ifp in the ng_ether detach callback" sentence.
Clarified for me.

I was referring to ng_ether_detach(). That callback is invoked when an ethernet interface with an attached ng_ether device is destroyed, and it destroys the ng_ether node.

sys/netgraph/ng_ether.c

760

That's correct.

[afedorov]

I don't really like how ng_etner(4) is implemented as a whole. But I think this patch is correct and should be committed.

[markj]

sys/netgraph/ng_base.c

3172

BTW:

• I am unsure why we do not just set NGF_REALLY_DIE before the first attempt.
• I don't know why the code doesn't use NG_NODE_REALLY_DIE().

[markj]

In D27662#618509, @afedorov wrote:

I don't really like how ng_etner(4) is implemented as a whole. But I think this patch is correct and should be committed.

Could you explain what you would change about it? I'm not familiar enough with ng_ether to know all of its warts.

[markj]

commit cd698c51790e956fed0975f451d3dfc361dc7c24