Re: [PATCH net v2] mctp i2c: fix double free of midev on netdev register failure

[Kenny Hsieh]

Hi Matt,

Sashiko found two problems in v2, and both come from removing
ndo_uninit, so I'd like to bring it back in v3, just without the free
this time.

It turns out ndo_uninit was also dropping sel and emptying tx_queue,
and it's the only hook the core calls after dev_close() but before
waiting for the last netdev ref. If NETDEV_REGISTER fails, sel's ref is
never dropped and register hangs. On a normal unregister, a skb left in
tx_queue can keep the netdev pinned.

So in v3 ndo_uninit just drops sel and empties tx_queue. Both are fine
to run twice, so freeing midev and the put_device()s still only happen
in mctp_i2c_midev_free(), from mctp_i2c_unregister() like you suggested.

I did look at keeping ndo_uninit out. That would mean only picking sel
after mctp_register_netdev() succeeds, and calling netif_tx_disable()
before emptying tx_queue in mctp_i2c_unregister(). It works around the
core's ordering rather than using the hook meant for it, and both bits
need a comment to explain why they're there. It also doesn't cover the
unlock marker, which release_flow() still queues after the purge. So
I'd rather go with ndo_uninit, but happy to switch if you prefer.

BR,
Kenny


The pre-existing one is in the mctp core rather than mctp-i2c, so I'll
leave it out of this fix.

pw-bot: cr