Re: [PATCH] eCryptFS: fix missed mutex_unlock

Previous thread: OCFS2 printf format warnings by Meelis Roos on Sunday, May 18, 2008 - 6:59 am. (2 messages)

Next thread: [PATCH] Make LIST_POISON less deadly by Avi Kivity on Sunday, May 18, 2008 - 8:38 am. (16 messages)
From: Cyrill Gorcunov
Date: Sunday, May 18, 2008 - 7:26 am

---

Ingo, could you please apply it and test? Actually I really doubt if it help
with the locking problem you pointed. There are two procedures
in miscrev.c - ecryptfs_miscdev_poll() and ecryptfs_miscdev_read()
which takes/releases mutexes in a bit strange way... investigating,
but this patch is needed anyway.

Index: linux-2.6.git/fs/ecryptfs/crypto.c
===================================================================
--- linux-2.6.git.orig/fs/ecryptfs/crypto.c	2008-05-18 16:44:20.000000000 +0400
+++ linux-2.6.git/fs/ecryptfs/crypto.c	2008-05-18 17:56:12.000000000 +0400
@@ -1903,6 +1903,7 @@ int ecryptfs_get_tfm_and_mutex_for_ciphe
 		if (rc) {
 			printk(KERN_ERR "Error adding new key_tfm to list; "
 					"rc = [%d]\n", rc);
+			mutex_unlock(&key_tfm_list_mutex);
 			goto out;
 		}
 	}
--

From: Andrew Morton
Date: Tuesday, May 20, 2008 - 12:28 am

Better to do it this way, I think:

--- a/fs/ecryptfs/crypto.c~ecryptfs-fix-missed-mutex_unlock
+++ a/fs/ecryptfs/crypto.c
@@ -1906,9 +1906,9 @@ int ecryptfs_get_tfm_and_mutex_for_ciphe
 			goto out;
 		}
 	}
-	mutex_unlock(&key_tfm_list_mutex);
 	(*tfm) = key_tfm->key_tfm;
 	(*tfm_mutex) = &key_tfm->key_tfm_mutex;
 out:
+	mutex_unlock(&key_tfm_list_mutex);
 	return rc;
 }
_

Holding the lock for an additional few instructions may not be strictly
needed, but we might avoid the reintroduction of such bugs?

--

From: Cyrill Gorcunov
Date: Tuesday, May 20, 2008 - 2:46 am

On Tue, May 20, 2008 at 11:28 AM, Andrew Morton

Good idea, thanks! Could you update the patch, please?
--

From: Ingo Molnar
Date: Wednesday, May 21, 2008 - 1:02 am

btw., i didnt do any specific ecryptfs testing - randconfig enabled it 
and it got booted. So if you dont get the warning during 
bootup/module-load, you'll have the same test coverage i did.

	Ingo
--

From: Cyrill Gorcunov
Date: Wednesday, May 21, 2008 - 2:33 am

oh, thanks ;)
--

Previous thread: OCFS2 printf format warnings by Meelis Roos on Sunday, May 18, 2008 - 6:59 am. (2 messages)

Next thread: [PATCH] Make LIST_POISON less deadly by Avi Kivity on Sunday, May 18, 2008 - 8:38 am. (16 messages)