On Thu, 28 Oct 2021 08:45:03 -0300 Jason Gunthorpe wrote: > > But will make all the callers of vlan_dev_real_dev() feel like they > > should NULL-check the result, which is not necessary. > > Isn't it better to reliably return NULL instead of a silent UAF in > this edge case? I don't know what the best practice is for maintaining sanity of unregistered objects. If there really is a requirement for the real_dev pointer to be sane we may want to move the put_device(real_dev) to vlan_dev_free(). There should not be any risk of circular dependency but I'm not 100% sure. > > RDMA must be calling this helper on a vlan which was already > > unregistered, can we fix RDMA instead? > > RDMA holds a get on the netdev which prevents unregistration, however > unregister_vlan_dev() does: > > unregister_netdevice_queue(dev, head); > dev_put(real_dev); > > Which corrupts the still registered vlan device while it is sitting in > the queue waiting to unregister. So, it is not true that a registered > vlan device always has working vlan_dev_real_dev(). That's not my reading, unless we have a different definition of "registered". The RDMA code in question runs from a workqueue, at the time the UNREGISTER notification is generated all objects are still alive and no UAF can happen. Past UNREGISTER extra care is needed when accessing the object. Note that unregister_vlan_dev() may queue the unregistration, without running it. If it clears real_dev the UNREGISTER notification will no longer be able to access real_dev, which used to be completely legal.
