Re: [PATCH] kobject: provide kobject_put_wait to fix module unloadrace

[Mikulas Patocka]

On Sun, 5 Jan 2014, Dmitry Torokhov wrote:

> > But sometimes, the driver itself needs to create nodes in the sysfs 
> > filesystem (for example drivers/md/dm-sysfs.c). I don't quite see how 
> > would you push all driver-specific sysfs nodes into the generic non-module 
> > code.
> 
> Then you need to make sure your driver does not allow unloading while
> its objects are active. I.e. require that all your devices are gone
> (by increasing module count when you create a DM object and decreasing
> it when you release DM object) before you allow unloading the driver.

For drivers that register to various subsystems (for example with 
pci_register_driver and pci_unregister_driver) this can't be done 
correctly - pci_unregister_driver is called from the module unload path, 
it destroys all instances of the device and their appropriate sysfs nodes.

The sysfs nodes exist even if the driver is unused and has zero module 
count.

> Basically we should avoid kobject_put() in exit paths of the module.



> Then we are left with that tiny race with release being preempted and
> module getting unloaded.

Majority of kobject users aren't managing module refcount in the 
release routine. They do not have a tiny race - they have a big race that 
is hapenning with CONFIG_DEBUG_KOBJECT_RELEASE.



These use completion to wait for the released object (thus, they are 
correct):
cpufreq_sysfs_release, cpuidle_sysfs_release, cpuidle_state_sysfs_release, 
cpuidle_driver_sysfs_release, ext4_sb_release, ext4_feat_release, 
f2fs_sb_release

These have no protection against module unload at all:
pkt_kobj_release, map_release, portio_release, ib_port_release, 
cm_release_port_obj, mlx4_port_release, ttm_bo_global_kobj_release, 
ttm_pool_kobj_release, ttm_mem_zone_kobj_release, 
ttm_mem_global_kobj_release, rdev_free, md_free, efivar_release, 
dmi_entry_free, dmi_sysfs_entry_release, edd_release, 
iscsi_boot_kobj_release, lockspace_kobj_release, gfs2_sbd_release, 
release_nbp

These have empty or non-existent release routine (thus having no 
protection): dm-sysfs.c, qib_port_release

These use module refcount: edac_device_ctrl_master_release, 
edac_device_ctrl_instance_release, edac_device_ctrl_block_release

> Thanks.
> 
> -- 
> Dmitry

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/