>From a19350097200570571aa522afebb96b34db534f4 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Thu, 14 Feb 2013 09:07:36 +0200 Subject: [PATCH] selinux: do not confuse lockdep Selinux has per-inode mutexes called 'isec->lock', and they are initialized in the same place, which makes lockdep treat all of the them as if they were identical. However, locking rules may be a little bit different depending on the file-system, so we should put these locks to separate classes, just like we do for 'i_mutex'. Namely, we should put them to per-FS type classes, which is exactly what this patch does. The problem this patch intends to fix is a strange lockdep warning, which I, frankly speaking, do not really understand, but I believe the root-cause should be fixed by this patch. Look at the stacktrace #4: here we have 'debugfs_create_dir()' [ 5.390312] ====================================================== [ 5.396500] [ INFO: possible circular locking dependency detected ] [ 5.402781] 3.8.0-rc6-00005-g4f7e39d #49 Not tainted [ 5.407750] ------------------------------------------------------- [ 5.414031] systemd/1 is trying to acquire lock: [ 5.418656] (&c->tnc_mutex){+.+...}, at: [] ubifs_tnc_locate+0x30/0x198 [ 5.426343] [ 5.426343] but task is already holding lock: [ 5.432218] (&isec->lock){+.+.+.}, at: [] inode_doinit_with_dentry+0x8c/0x55c [ 5.440375] [ 5.440375] which lock already depends on the new lock. [ 5.440375] [ 5.448593] [ 5.448593] the existing dependency chain (in reverse order) is: [ 5.456093] -> #4 (&isec->lock){+.+.+.}: [ 5.460250] [] lock_acquire+0x64/0x78 [ 5.465437] [] mutex_lock_nested+0x5c/0x2ec [ 5.471125] [] inode_doinit_with_dentry+0x8c/0x55c [ 5.477437] [] security_d_instantiate+0x1c/0x34 [ 5.483500] [] debugfs_mknod.part.15.constprop.18+0x94/0x128 [ 5.490656] [] __create_file+0x1b0/0x25c [ 5.496093] [] debugfs_create_dir+0x1c/0x28 [ 5.501781] [] pinctrl_init+0x1c/0xd0 [ 5.506968] [] do_one_initcall+0x108/0x17c [ 5.512593] [] kernel_init_freeable+0xec/0x1b4 [ 5.518562] [] kernel_init+0x8/0xe4 [ 5.523562] [] ret_from_fork+0x14/0x2c [ 5.528812] -> #3 (&sb->s_type->i_mutex_key#2){+.+.+.}: [ 5.534312] [] lock_acquire+0x64/0x78 [ 5.539468] [] mutex_lock_nested+0x5c/0x2ec [ 5.545187] [] __create_file+0x50/0x25c [ 5.550531] [] debugfs_create_dir+0x1c/0x28 [ 5.556218] [] clk_debug_create_subtree+0x1c/0x108 [ 5.562500] [] clk_debug_init+0x68/0xc0 [ 5.567875] [] do_one_initcall+0x108/0x17c [ 5.573468] [] kernel_init_freeable+0xec/0x1b4 [ 5.579437] [] kernel_init+0x8/0xe4 [ 5.584437] [] ret_from_fork+0x14/0x2c [ 5.589687] -> #2 (prepare_lock){+.+.+.}: [ 5.593937] [] lock_acquire+0x64/0x78 [ 5.599125] [] mutex_lock_nested+0x5c/0x2ec [ 5.604812] [] clk_prepare+0x18/0x38 [ 5.609906] [] __gpmi_enable_clk+0x30/0xb0 [ 5.615531] [] gpmi_begin+0x18/0x530 [ 5.620625] [] gpmi_select_chip+0x3c/0x54 [ 5.626156] [] nand_do_read_ops+0x7c/0x3e4 [ 5.631750] [] nand_read+0x50/0x74 [ 5.636656] [] part_read+0x5c/0xa4 [ 5.641593] [] mtd_read+0x84/0xb8 [ 5.646406] [] ubi_io_read+0xa0/0x2c0 [ 5.651593] [] ubi_eba_read_leb+0x190/0x424 [ 5.657281] [] ubi_leb_read+0xac/0x120 [ 5.662562] [] ubifs_leb_read+0x28/0x8c [ 5.667906] [] ubifs_read_node+0x98/0x2a0 [ 5.673437] [] ubifs_read_sb_node+0x54/0x78 [ 5.679125] [] ubifs_read_superblock+0xc60/0x163c [ 5.685343] [] ubifs_mount+0x800/0x171c [ 5.690687] [] mount_fs+0x44/0x184 [ 5.695593] [] vfs_kern_mount+0x4c/0xc0 [ 5.700968] [] do_mount+0x18c/0x8d0 [ 5.705968] [] sys_mount+0x84/0xb8 [ 5.710875] [] mount_block_root+0x118/0x258 [ 5.716562] [] prepare_namespace+0x8c/0x17c [ 5.722281] [] kernel_init+0x8/0xe4 [ 5.727281] [] ret_from_fork+0x14/0x2c [ 5.732531] -> #1 (&le->mutex){++++..}: [ 5.736625] [] lock_acquire+0x64/0x78 [ 5.741812] [] down_read+0x40/0x54 [ 5.746718] [] ubi_eba_read_leb+0x34/0x424 [ 5.752312] [] ubi_leb_read+0xac/0x120 [ 5.757562] [] ubifs_leb_read+0x28/0x8c [ 5.762937] [] ubifs_read_node+0x98/0x2a0 [ 5.768437] [] ubifs_load_znode+0x88/0x560 [ 5.774062] [] ubifs_lookup_level0+0x190/0x1dc [ 5.780031] [] ubifs_tnc_locate+0x44/0x198 [ 5.785656] [] ubifs_iget+0x6c/0x8a4 [ 5.790718] [] ubifs_mount+0xc18/0x171c [ 5.796093] [] mount_fs+0x44/0x184 [ 5.801000] [] vfs_kern_mount+0x4c/0xc0 [ 5.806343] [] do_mount+0x18c/0x8d0 [ 5.811343] [] sys_mount+0x84/0xb8 [ 5.816250] [] mount_block_root+0x118/0x258 [ 5.821968] [] prepare_namespace+0x8c/0x17c [ 5.827656] [] kernel_init+0x8/0xe4 [ 5.832656] [] ret_from_fork+0x14/0x2c [ 5.837906] -> #0 (&c->tnc_mutex){+.+...}: [ 5.842250] [] __lock_acquire+0x14ec/0x1b08 [ 5.847968] [] lock_acquire+0x64/0x78 [ 5.853125] [] mutex_lock_nested+0x5c/0x2ec [ 5.858812] [] ubifs_tnc_locate+0x30/0x198 [ 5.864437] [] ubifs_tnc_lookup_nm+0x28/0x150 [ 5.870312] [] ubifs_getxattr+0xc0/0x254 [ 5.875750] [] inode_doinit_with_dentry+0x25c/0x55c [ 5.882125] [] sb_finish_set_opts+0xa4/0x21c [ 5.887906] [] selinux_set_mnt_opts+0x2ec/0x4b8 [ 5.893968] [] superblock_doinit+0xb4/0xc0 [ 5.899562] [] iterate_supers+0xb4/0xdc [ 5.904906] [] security_load_policy+0x248/0x3c4 [ 5.910968] [] sel_write_load+0x9c/0x6a0 [ 5.916406] [] vfs_write+0xa0/0x17c [ 5.921406] [] sys_write+0x3c/0x70 [ 5.926343] [] ret_fast_syscall+0x0/0x38 [ 5.931781] [ 5.931781] other info that might help us debug this: [ 5.931781] [ 5.939781] Chain exists of: &c->tnc_mutex --> &sb->s_type->i_mutex_key#2 --> &isec->lock [ 5.948500] Possible unsafe locking scenario: [ 5.948500] [ 5.954437] CPU0 CPU1 [ 5.958968] ---- ---- [ 5.963500] lock(&isec->lock); [ 5.966781] lock(&sb->s_type->i_mutex_key#2); [ 5.973843] lock(&isec->lock); [ 5.979625] lock(&c->tnc_mutex); [ 5.983062] [ 5.983062] *** DEADLOCK *** [ 5.983062] [ 5.989000] 4 locks held by systemd/1: [ 5.992781] #0: (sel_mutex){+.+.+.}, at: [] sel_write_load+0x20/0x6a0 [ 6.000343] #1: (&type->s_umount_key#26){.+.+..}, at: [] iterate_supers+0x90/0xdc [ 6.008968] #2: (&sbsec->lock){+.+.+.}, at: [] selinux_set_mnt_opts+0x64/0x4b8 [ 6.017343] #3: (&isec->lock){+.+.+.}, at: [] inode_doinit_with_dentry+0x8c/0x55c [ 6.025937] [ 6.025937] stack backtrace: [ 6.030375] [] (unwind_backtrace+0x0/0xf0) from [] (print_circular_bug+0x25c/0x2a8) [ 6.039843] [] (print_circular_bug+0x25c/0x2a8) from [] (__lock_acquire+0x14ec/0x1b08) [ 6.049531] [] (__lock_acquire+0x14ec/0x1b08) from [] (lock_acquire+0x64/0x78) [ 6.058531] [] (lock_acquire+0x64/0x78) from [] (mutex_lock_nested+0x5c/0x2ec) [ 6.067531] [] (mutex_lock_nested+0x5c/0x2ec) from [] (ubifs_tnc_locate+0x30/0x198) [ 6.076968] [] (ubifs_tnc_locate+0x30/0x198) from [] (ubifs_tnc_lookup_nm+0x28/0x150) [ 6.086593] [] (ubifs_tnc_lookup_nm+0x28/0x150) from [] (ubifs_getxattr+0xc0/0x254) [ 6.096031] [] (ubifs_getxattr+0xc0/0x254) from [] (inode_doinit_with_dentry+0x25c/0x55c) [ 6.105968] [] (inode_doinit_with_dentry+0x25c/0x55c) from [] (sb_finish_set_opts+0xa4/0x21c) [ 6.116281] [] (sb_finish_set_opts+0xa4/0x21c) from [] (selinux_set_mnt_opts+0x2ec/0x4b8) [ 6.126218] [] (selinux_set_mnt_opts+0x2ec/0x4b8) from [] (superblock_doinit+0xb4/0xc0) [ 6.136000] [] (superblock_doinit+0xb4/0xc0) from [] (iterate_supers+0xb4/0xdc) [ 6.145093] [] (iterate_supers+0xb4/0xdc) from [] (security_load_policy+0x248/0x3c4) [ 6.154625] [] (security_load_policy+0x248/0x3c4) from [] (sel_write_load+0x9c/0x6a0) [ 6.164218] [] (sel_write_load+0x9c/0x6a0) from [] (vfs_write+0xa0/0x17c) [ 6.172781] [] (vfs_write+0xa0/0x17c) from [] (sys_write+0x3c/0x70) [ 6.180843] [] (sys_write+0x3c/0x70) from [] (ret_fast_syscall+0x0/0x38) Reported-by: Marc Kleine-Budde Signed-off-by: Artem Bityutskiy --- include/linux/fs.h | 1 + security/selinux/hooks.c | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 7617ee0..ccd49e1 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1824,6 +1824,7 @@ struct file_system_type { struct lock_class_key i_lock_key; struct lock_class_key i_mutex_key; struct lock_class_key i_mutex_dir_key; + struct lock_class_key i_security_lock_key; }; extern struct dentry *mount_ns(struct file_system_type *fs_type, int flags, diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ef26e96..13ccbb0 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -207,6 +207,8 @@ static int inode_alloc_security(struct inode *inode) return -ENOMEM; mutex_init(&isec->lock); + lockdep_set_class(&isec->lock, + &inode->i_sb->s_type->i_security_lock_key); INIT_LIST_HEAD(&isec->list); isec->inode = inode; isec->sid = SECINITSID_UNLABELED; -- 1.7.7.6