[RHEL7,COMMIT] ms/dm crypt: replace RCU read-side section with rwsem

Submitted by Konstantin Khorenko on Feb. 5, 2018, 10:24 a.m.

Details

Message ID 201802051024.w15AOnYr005619@finist_ce7.work
State New
Series "ms/dm crypt: replace RCU read-side section with rwsem"
Headers show

Commit Message

Konstantin Khorenko Feb. 5, 2018, 10:24 a.m.
The commit is pushed to "branch-rh7-3.10.0-693.17.1.vz7.43.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-693.17.1.vz7.43.1
------>
commit d9a9237dd1f81c553b3e32466367ec093fabd2fd
Author: Konstantin Khorenko <khorenko@virtuozzo.com>
Date:   Mon Feb 5 12:44:47 2018 +0300

    ms/dm crypt: replace RCU read-side section with rwsem
    
    This is a backport of 2 ms patches:
    
      f5b0cba8f239 ("dm crypt: replace RCU read-side section with rwsem)"
      0837e49ab3fa ("KEYS: Differentiate uses of rcu_dereference_key() and
                     user_key_payload())"
    
    commit f5b0cba8f23915e92932f11eb063e37d70556a89
    Author: Ondrej Kozina <okozina@redhat.com>
    Date:   Tue Jan 31 15:47:11 2017 +0100
    
        dm crypt: replace RCU read-side section with rwsem
    
        The lockdep splat below hints at a bug in RCU usage in dm-crypt that
        was introduced with commit c538f6ec9f56 ("dm crypt: add ability to use
        keys from the kernel key retention service").  The kernel keyring
        function user_key_payload() is in fact a wrapper for
        rcu_dereference_protected() which must not be called with only
        rcu_read_lock() section mark.
    
        Unfortunately the kernel keyring subsystem doesn't currently provide
        an interface that allows the use of an RCU read-side section.  So for
        now we must drop RCU in favour of rwsem until a proper function is
        made available in the kernel keyring subsystem.
    
        ===============================
        [ INFO: suspicious RCU usage. ]
        4.10.0-rc5 #2 Not tainted
        -------------------------------
        ./include/keys/user-type.h:53 suspicious rcu_dereference_protected() usage!
        other info that might help us debug this:
        rcu_scheduler_active = 2, debug_locks = 1
        2 locks held by cryptsetup/6464:
         #0:  (&md->type_lock){+.+.+.}, at: [<ffffffffa02472a2>] dm_lock_md_type+0x12/0x20 [dm_mod]
         #1:  (rcu_read_lock){......}, at: [<ffffffffa02822f8>] crypt_set_key+0x1d8/0x4b0 [dm_crypt]
        stack backtrace:
        CPU: 1 PID: 6464 Comm: cryptsetup Not tainted 4.10.0-rc5 #2
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.1-1.fc24 04/01/2014
        Call Trace:
         dump_stack+0x67/0x92
         lockdep_rcu_suspicious+0xc5/0x100
         crypt_set_key+0x351/0x4b0 [dm_crypt]
         ? crypt_set_key+0x1d8/0x4b0 [dm_crypt]
         crypt_ctr+0x341/0xa53 [dm_crypt]
         dm_table_add_target+0x147/0x330 [dm_mod]
         table_load+0x111/0x350 [dm_mod]
         ? retrieve_status+0x1c0/0x1c0 [dm_mod]
         ctl_ioctl+0x1f5/0x510 [dm_mod]
         dm_ctl_ioctl+0xe/0x20 [dm_mod]
         do_vfs_ioctl+0x8e/0x690
         ? ____fput+0x9/0x10
         ? task_work_run+0x7e/0xa0
         ? trace_hardirqs_on_caller+0x122/0x1b0
         SyS_ioctl+0x3c/0x70
         entry_SYSCALL_64_fastpath+0x18/0xad
        RIP: 0033:0x7f392c9a4ec7
        RSP: 002b:00007ffef6383378 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
        RAX: ffffffffffffffda RBX: 00007ffef63830a0 RCX: 00007f392c9a4ec7
        RDX: 000000000124fcc0 RSI: 00000000c138fd09 RDI: 0000000000000005
        RBP: 00007ffef6383090 R08: 00000000ffffffff R09: 00000000012482b0
        R10: 2a28205d34383336 R11: 0000000000000246 R12: 00007f392d803a08
        R13: 00007ffef63831e0 R14: 0000000000000000 R15: 00007f392d803a0b
    
        Fixes: c538f6ec9f56 ("dm crypt: add ability to use keys from the kernel key retention service")
        Reported-by: Milan Broz <mbroz@redhat.com>
        Signed-off-by: Ondrej Kozina <okozina@redhat.com>
        Reviewed-by: Mikulas Patocka <mpatocka@redhat.com>
        Signed-off-by: Mike Snitzer <snitzer@redhat.com>
    
    =====================================================
    commit 0837e49ab3fa8d903a499984575d71efee8097ce
    Author: David Howells <dhowells@redhat.com>
    Date:   Wed Mar 1 15:11:23 2017 +0000
    
        KEYS: Differentiate uses of rcu_dereference_key() and user_key_payload()
    
        rcu_dereference_key() and user_key_payload() are currently being used in
        two different, incompatible ways:
    
         (1) As a wrapper to rcu_dereference() - when only the RCU read lock used
             to protect the key.
    
         (2) As a wrapper to rcu_dereference_protected() - when the key semaphor is
             used to protect the key and the may be being modified.
    
        Fix this by splitting both of the key wrappers to produce:
    
         (1) RCU accessors for keys when caller has the key semaphore locked:
    
                dereference_key_locked()
                user_key_payload_locked()
    
         (2) RCU accessors for keys when caller holds the RCU read lock:
    
                dereference_key_rcu()
                user_key_payload_rcu()
    
        This should fix following warning in the NFS idmapper
    
          ===============================
          [ INFO: suspicious RCU usage. ]
          4.10.0 #1 Tainted: G        W
          -------------------------------
          ./include/keys/user-type.h:53 suspicious rcu_dereference_protected() usage!
          other info that might help us debug this:
          rcu_scheduler_active = 2, debug_locks = 0
          1 lock held by mount.nfs/5987:
            #0:  (rcu_read_lock){......}, at: [<d000000002527abc>] nfs_idmap_get_key+0x15c/0x420 [nfsv4]
          stack backtrace:
          CPU: 1 PID: 5987 Comm: mount.nfs Tainted: G        W       4.10.0 #1
          Call Trace:
            dump_stack+0xe8/0x154 (unreliable)
            lockdep_rcu_suspicious+0x140/0x190
            nfs_idmap_get_key+0x380/0x420 [nfsv4]
            nfs_map_name_to_uid+0x2a0/0x3b0 [nfsv4]
            decode_getfattr_attrs+0xfac/0x16b0 [nfsv4]
            decode_getfattr_generic.constprop.106+0xbc/0x150 [nfsv4]
            nfs4_xdr_dec_lookup_root+0xac/0xb0 [nfsv4]
            rpcauth_unwrap_resp+0xe8/0x140 [sunrpc]
            call_decode+0x29c/0x910 [sunrpc]
            __rpc_execute+0x140/0x8f0 [sunrpc]
            rpc_run_task+0x170/0x200 [sunrpc]
            nfs4_call_sync_sequence+0x68/0xa0 [nfsv4]
            _nfs4_lookup_root.isra.44+0xd0/0xf0 [nfsv4]
            nfs4_lookup_root+0xe0/0x350 [nfsv4]
            nfs4_lookup_root_sec+0x70/0xa0 [nfsv4]
            nfs4_find_root_sec+0xc4/0x100 [nfsv4]
            nfs4_proc_get_rootfh+0x5c/0xf0 [nfsv4]
            nfs4_get_rootfh+0x6c/0x190 [nfsv4]
            nfs4_server_common_setup+0xc4/0x260 [nfsv4]
            nfs4_create_server+0x278/0x3c0 [nfsv4]
            nfs4_remote_mount+0x50/0xb0 [nfsv4]
            mount_fs+0x74/0x210
            vfs_kern_mount+0x78/0x220
            nfs_do_root_mount+0xb0/0x140 [nfsv4]
            nfs4_try_mount+0x60/0x100 [nfsv4]
            nfs_fs_mount+0x5ec/0xda0 [nfs]
            mount_fs+0x74/0x210
            vfs_kern_mount+0x78/0x220
            do_mount+0x254/0xf70
            SyS_mount+0x94/0x100
            system_call+0x38/0xe0
    
        Reported-by: Jan Stancek <jstancek@redhat.com>
        Signed-off-by: David Howells <dhowells@redhat.com>
        Tested-by: Jan Stancek <jstancek@redhat.com>
        Signed-off-by: James Morris <james.l.morris@oracle.com>
    
    Fixes: compilation fix for
       935db223f5e6 ("dm-crypt: add ability to use keys from the kernel key
                      retention service")
    
    Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 drivers/md/dm-crypt.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index a8c17353e057..247a66804dd4 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1532,15 +1532,15 @@  static int crypt_set_keyring_key(struct crypt_config *cc, char *key_desc)
 	if (IS_ERR(key))
 		return PTR_ERR(key);
 
-	rcu_read_lock();
-	ukp = user_key_payload(key);
+	down_read(&key->sem);
+	ukp = user_key_payload_locked(key);
 	if (cc->key_size != ukp->datalen) {
 		ret = -EINVAL;
 		goto out;
 	}
 	memcpy(cc->key, ukp->data, cc->key_size);
 out:
-	rcu_read_unlock();
+	up_read(&key->sem);
 	key_put(key);
 	return ret;
 }
@@ -1557,9 +1557,9 @@  static int get_key_size(char *key_desc)
 	if (IS_ERR(key))
 		return PTR_ERR(key);
 
-	rcu_read_lock();
-	ret = user_key_payload(key)->datalen;
-	rcu_read_unlock();
+	down_read(&key->sem);
+	ret = user_key_payload_locked(key)->datalen;
+	up_read(&key->sem);
 	key_put(key);
 	return ret;
 }