linux-kernel - [PATCH] eCryptfs: Remove useless lock

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: <20080521153915.GA942@halcrowt61p.austin.ibm.com>
Date:	Wed, 21 May 2008 10:39:15 -0500
From:	Michael Halcrow <mhalcrow@...ibm.com>
To:	Andrea Righi <righiandr@...rs.sourceforge.net>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Rusty Russell <prussell@....ibm.com>,
	"David J. Kleikamp" <shaggy@...ibm.com>,
	"SERGE E. HALLYN" <sergeh@...ibm.com>
Subject: [PATCH] eCryptfs: Remove useless lock

On Wed, May 21, 2008 at 10:14:30AM -0500, Michael Halcrow wrote:
> On Wed, May 21, 2008 at 01:16:15AM +0200, Andrea Righi wrote:
> > Michael Halcrow wrote:
> >> +	req = kmem_cache_alloc(ecryptfs_open_req_cache, GFP_KERNEL);
> >> +	if (!req) {
> >> +		rc = -ENOMEM;
> >> +		goto out;
> >> +	}
> >> +	mutex_init(&req->mux);
> >> +	req->lower_file = lower_file;
> >> +	req->lower_dentry = lower_dentry;
> >> +	req->lower_mnt = lower_mnt;
> >> +	req->requesting_task = current;
> >> +	req->flags = 0;
> >> +	mutex_lock(&ecryptfs_kthread_ctl.mux);
> >> +	mutex_lock(&req->mux);
> >> +	if (ecryptfs_kthread_ctl.flags & ECRYPTFS_KTHREAD_ZOMBIE) {
> >> +		rc = -EIO;
> >
> > Isn't a mutex_unlock(&req->mux) missing here?
> 
> Nobody else actually has a reference to req at this point, so the
> unlock is not really necessary. It might not be a bad idea to jump to
> out_unlock anyway just to avoid future bugs.
> 
> >> +		mutex_unlock(&ecryptfs_kthread_ctl.mux);
> >> +		printk(KERN_ERR "%s: We are in the middle of shutting down; "
> >> +		       "aborting privileged request to open lower file\n",
> >> +			__func__);
> >> +		goto out_free;
> >> +	}
> >> +	list_add_tail(&req->kthread_ctl_list, &ecryptfs_kthread_ctl.req_list);
> >> +	mutex_unlock(&req->mux);
> >> +	mutex_unlock(&ecryptfs_kthread_ctl.mux);

Nobody can get a reference to req so long as &ecryptfs_kthread_ctl.mux
is held. Remove lock of req->mux for newly created req.

Signed-off-by: Michael Halcrow <mhalcrow@...ibm.com>
---
 fs/ecryptfs/kthread.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/fs/ecryptfs/kthread.c b/fs/ecryptfs/kthread.c
index dba7c67..3d34327 100644
--- a/fs/ecryptfs/kthread.c
+++ b/fs/ecryptfs/kthread.c
@@ -158,7 +158,6 @@ int ecryptfs_privileged_open(struct file **lower_file,
 	req->requesting_task = current;
 	req->flags = 0;
 	mutex_lock(&ecryptfs_kthread_ctl.mux);
-	mutex_lock(&req->mux);
 	if (ecryptfs_kthread_ctl.flags & ECRYPTFS_KTHREAD_ZOMBIE) {
 		rc = -EIO;
 		mutex_unlock(&ecryptfs_kthread_ctl.mux);
@@ -168,7 +167,6 @@ int ecryptfs_privileged_open(struct file **lower_file,
 		goto out_free;
 	}
 	list_add_tail(&req->kthread_ctl_list, &ecryptfs_kthread_ctl.req_list);
-	mutex_unlock(&req->mux);
 	mutex_unlock(&ecryptfs_kthread_ctl.mux);
 	wake_up(&ecryptfs_kthread_ctl.wait);
 schedule:
-- 
1.5.3.6

--
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/