svn commit: r303870 - head/sys/dev/mlx5/mlx5_en

[Hans Petter Selasky]

On 08/09/16 19:25, John Baldwin wrote:
> On Tuesday, August 09, 2016 07:43:15 AM Hans Petter Selasky wrote:
>> Author: hselasky
>> Date: Tue Aug  9 07:43:15 2016
>> New Revision: 303870
>> URL: https://svnweb.freebsd.org/changeset/base/303870
>>
>> Log:
>>   Fix for use after free.
>>
>>   Clear the device description to avoid use after free because the
>>   bsddev is not destroyed when the mlx5en module is unloaded. Only when
>>   the parent mlx5 module is unloaded the bsddev is destroyed. This fixes
>>   a panic on listing sysctls which refer strings in the bsddev after the
>>   mlx5en module has been unloaded.
>>
>>   Sponsored by:	Mellanox Technologies
>>   MFC after:	1 week
>
> Hmmm, this seems like it is working around a bug somewhere else.
> device_detach() calls device_set_driver(dev, NULL) which in turn calls
> device_set_desc(dev, NULL) which should be clearing the description.  You can
> only be leaking a desc pointer if you aren't detaching the device.  Not
> detaching a device but unloading the module containing part (but apparently
> not all) of its driver would seem to be fraught with peril.  Why are you not
> detaching the mlx5en0 device when unloading this module?
>

Hi John,

It is not a bug in the kernel.

When mlx5en is unloaded, device_detach() is not called, and that is 
expected. The mlx5 and mlx4 family of drivers have their own one-level 
bus subsystem. mlx5.ko will call LINUXKPI's pci_register_driver() and 
then probe mlx5en internally. When mlx5en is detached, mlx5 will detach 
the mlx5en driver, but it will not call "pci_unregister_driver()" which 
calls the device_detach(). This will only happen when the mlx5.ko is 
unloaded. Because the mlx5, mlx5en and mlx5ib (coming) modules are 
separated we can end up in this situation.

I hope you understand and that my explanation was not too complicated.

For other in-kernel drivers, this is not a problem. Like you write 
device_detach() will take care of device_set_driver(dev, NULL) and that 
will clear the device description.

--HPS