Re: [PATCH net-next 1/2] net:dsa: fix dsa_dst_del_ds()

Re: [PATCH net-next 1/2] net:dsa: fix dsa_dst_del_ds()

[Andrew Lunn]

On Wed, Dec 07, 2016 at 05:53:05PM +0100, Volodymyr Bendiuga wrote:
> When dsa_dst_del_ds() is called, do not free the whole list,
> instead, only decrement refcount for the switch tree. The list
> will be deleted in dsa_put_dst() if refcount is 0. Nothing
> really needs to be freed for dsa switch, therefore dsa_free_ds()
> is empty. kref_put() will print warning if dsa_free_ds() is not
> passed as a parameter to it.

This does not look correct. I would expect there to be some symmetry.

The dst gets allocated in _dsa_register_switch(), so it should be
freed somewhere in or under _dsa_unregister_switch(). As you say, it
can be freed from dsa_free_dst(), but that is not called from
_dsa_unregister_switch().

dsa_dst_add_ds() and dsa_dst_del_ds() currently look symmetric.  Add
increments the ref count for the tree, del decrements it. When it
reaches zero, the tree is freed. 

dsa_dst_del_ds() is called from _dsa_unregister_switch(), which gives
us the symmetry with _dsa_register_switch().

What problem are you actually seeing? A double free? A use after free?

Thanks
	Andrew