linux-kernel - Re: [PATCH] kobject: provide kobject_put

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LRH.2.02.1401061303300.6190@file01.intranet.prod.int.rdu2.redhat.com>
Date:	Mon, 6 Jan 2014 13:43:07 -0500 (EST)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
cc:	Al Viro <viro@...IV.linux.org.uk>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jeff Mahoney <jeffm@...e.com>, torvalds@...ux-foundation.org,
	linux-kernel@...r.kernel.org, dm-devel@...hat.com,
	tglx@...utronix.de, paulmck@...ux.vnet.ibm.com, mingo@...nel.org
Subject: Re: [PATCH] kobject: provide kobject_put_wait to fix module unload
 race



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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/