Re: [PATCH] wifi: nl80211: serialize socket owner release with netdev teardown

[Cen Zhang]

Hi Johannes,

Thanks for your review and comments.

> What's that supposed to mean? Of course there's ordering between those
> two things?

You're right, that wording was imprecise.

I didn't mean that NETDEV_GOING_DOWN and NETDEV_UNREGISTER are unordered with
respect to each other. What I meant is that the NETLINK_URELEASE notifier's
schedule_work() site is not ordered against the teardown-side
cancel_work_sync() for the same wdev.

> Yeah I guess that seems like it could happen.

Yes, that is the race I was trying to describe:

  notifier observes conn_owner_nlportid
  teardown clears the connection state and cancel_work_sync() returns
  notifier then queues disconnect_wk
  unregister/free can then happen before the queued work runs

> That seems a bit over the top vs. just moving the cancel - why is this
> the only correct fix?

Agreed, I should not have implied that RTNL is the only correct fix.

I chose RTNL because netdev notifier delivery is already serialized by RTNL,
so it orders the existing schedule_work() producer against the existing
cancel_work_sync() with a small change. But if you prefer moving the cancel,
I can rework v2 in that direction.

My only concern with simply moving a final cancel into
_cfg80211_unregister_wdev() is that this path is called with the wiphy mutex
held, while cfg80211_autodisconnect_wk() also takes the wiphy mutex. So I
think the final drain would need to happen after the RCU readers are gone,
but outside the wiphy lock and before the wdev/netdev lifetime can end.

Best regards,
Cen Zhang