[PATCH 1/5] autofs4 - fix symlink name allocation

[PATCH 1/5] autofs4 - fix symlink name allocation

[Ian Kent]

The length of the symlink name has been moved but it needs to be
set before allocating space for it in the dentry info struct.
This corrects a mistake in a recent patch.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 fs/autofs4/root.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 7f3ebf1..10add99 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -774,6 +774,7 @@ static int autofs4_dir_symlink(struct inode *dir,
 		list_del_init(&ino->active);
 	spin_unlock(&sbi->lookup_lock);
 
+	ino->size = strlen(symname);
 	cp = kmalloc(ino->size + 1, GFP_KERNEL);
 	if (!cp) {
 		if (!dentry->d_fsdata)
@@ -805,7 +806,6 @@ static int autofs4_dir_symlink(struct inode *dir,
 		atomic_inc(&p_ino->count);
 	ino->inode = inode;
 
-	ino->size = strlen(symname);
 	ino->u.symlink = cp;
 	dir->i_mtime = CURRENT_TIME;
 

[PATCH 2/5] autofs4 - dont make expiring dentry negative fix

[Ian Kent]

The correction to not make a dentry negative has an error.
Currently ->lookup() checks if an expire is in progress and removes
the entry from the list, before the expire is complete so if more
than one process is performing a lookup at this time only one will
wait for expire completion. We need to not remove the list entry
until after waiting for expire completion.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 fs/autofs4/root.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)


diff --git a/fs/autofs4/root.c b/fs/autofs4/root.c
index 10add99..2a1a631 100644
--- a/fs/autofs4/root.c
+++ b/fs/autofs4/root.c
@@ -591,7 +591,6 @@ static struct dentry *autofs4_lookup_expiring(struct autofs_sb_info *sbi, struct
 			goto next;
 
 		if (d_unhashed(dentry)) {
-			list_del_init(&ino->expiring);
 			dget(dentry);
 			spin_unlock(&dentry->d_lock);
 			spin_unlock(&sbi->lookup_lock);
@@ -643,6 +642,10 @@ static struct dentry *autofs4_lookup(struct inode *dir, struct dentry *dentry, s
 			autofs4_wait(sbi, expiring, NFY_NONE);
 			DPRINTK("request completed");
 		}
+		spin_lock(&sbi->lookup_lock);
+		if (!list_empty(&ino->expiring))
+			list_del_init(&ino->expiring);
+		spin_unlock(&sbi->lookup_lock);
 		dput(expiring);
 	}
 

[PATCH 3/5] autofs4 - fix waitq locking

[Ian Kent]

The autofs4_catatonic_mode() function accesses the wait queue without
any locking but can be called at any time. This could lead to a
possible double free of the name field of the wait and a double fput
of the daemon communication pipe or an fput of a NULL file pointer.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 fs/autofs4/inode.c |    4 ++--
 fs/autofs4/waitq.c |   23 ++++++++++++-----------
 2 files changed, 14 insertions(+), 13 deletions(-)


diff --git a/fs/autofs4/inode.c b/fs/autofs4/inode.c
index e3e7099..7bb3e5b 100644
--- a/fs/autofs4/inode.c
+++ b/fs/autofs4/inode.c
@@ -163,8 +163,8 @@ void autofs4_kill_sb(struct super_block *sb)
 	if (!sbi)
 		goto out_kill_sb;
 
-	if (!sbi->catatonic)
-		autofs4_catatonic_mode(sbi); /* Free wait queues, close pipe */
+	/* Free wait queues, close pipe */
+	autofs4_catatonic_mode(sbi);
 
 	/* Clean up and release dangling references */
 	autofs4_force_release(sbi);
diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index d291b07..f8dee31 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -28,6 +28,12 @@ void autofs4_catatonic_mode(struct autofs_sb_info *sbi)
 {
 	struct autofs_wait_queue *wq, *nwq;
 
+	mutex_lock(&sbi->wq_mutex);
+	if (sbi->catatonic) {
+		mutex_unlock(&sbi->wq_mutex);
+		return;
+	}
+
 	DPRINTK("entering catatonic mode");
 
 	sbi->catatonic = 1;
@@ -45,6 +51,8 @@ void autofs4_catatonic_mode(struct autofs_sb_info *sbi)
 	}
 	fput(sbi->pipe);	/* Close the pipe */
 	sbi->pipe = NULL;
+	sbi->pipefd = -1;
+	mutex_unlock(&sbi->wq_mutex);
 }
 
 static int autofs4_write(struct file *file, const void *addr, int bytes)
@@ -398,17 +406,10 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
 			wq->name.name, notify);
 	}
 
-	/* wq->name is NULL if and only if the lock is already released */
-
-	if (sbi->catatonic) {
-		/* We might have slept, so check again for catatonic mode */
-		wq->status = -ENOENT;
-		if (wq->name.name) {
-			kfree(wq->name.name);
-			wq->name.name = NULL;
-		}
-	}
-
+	/*
+	 * wq->name.name is NULL iff the lock is already released
+	 * or the mount has been made catatonic.
+	 */
 	if (wq->name.name) {
 		/* Block all but "shutdown" signals while waiting */
 		sigset_t oldset;

[PATCH 4/5] autofs4 - check kernel communication pipe is valid for write

[Ian Kent]

It is possible for an autofs mount to become catatonic (and for the
daemon communication pipe to become NULL) after a wait has been
initiallized but before the request has been sent to the daemon. We
need to check for this before sending the request packet.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 fs/autofs4/waitq.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)


diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index f8dee31..aa7b526 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -99,6 +99,7 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
 		union autofs_packet_union v4_pkt;
 		union autofs_v5_packet_union v5_pkt;
 	} pkt;
+	struct file *pipe = NULL;
 	size_t pktsz;
 
 	DPRINTK("wait id = 0x%08lx, name = %.*s, type=%d",
@@ -164,8 +165,19 @@ static void autofs4_notify_daemon(struct autofs_sb_info *sbi,
 		return;
 	}
 
-	if (autofs4_write(sbi->pipe, &pkt, pktsz))
-		autofs4_catatonic_mode(sbi);
+	/* Check if we have become catatonic */
+	mutex_lock(&sbi->wq_mutex);
+	if (!sbi->catatonic) {
+		pipe = sbi->pipe;
+		get_file(pipe);
+	}
+	mutex_unlock(&sbi->wq_mutex);
+
+	if (pipe) {
+		if (autofs4_write(pipe, &pkt, pktsz))
+			autofs4_catatonic_mode(sbi);
+		fput(pipe);
+	}
 }
 
 static int autofs4_getpath(struct autofs_sb_info *sbi,

[PATCH 5/5] autofs4 - fix waitq memory leak

[Ian Kent]

If an autofs mount becomes catatonic before autofs4_wait_release() is
called the wait queue counter will not be decremented down to zero
and the entry will never be freed. There are also races decrementing
the wait counter in the wait release function. To deal with this the
counter needs to be updated while holding the wait queue mutex and
waiters need to be woken up unconditionally when the wait is removed
from the queue to ensure we eventually free the wait.

Signed-off-by: Ian Kent <raven@themaw.net>

---

 fs/autofs4/autofs_i.h |    2 +-
 fs/autofs4/waitq.c    |   18 +++++++++---------
 2 files changed, 10 insertions(+), 10 deletions(-)


diff --git a/fs/autofs4/autofs_i.h b/fs/autofs4/autofs_i.h
index da8882f..058e180 100644
--- a/fs/autofs4/autofs_i.h
+++ b/fs/autofs4/autofs_i.h
@@ -84,7 +84,7 @@ struct autofs_wait_queue {
 	pid_t tgid;
 	/* This is for status reporting upon return */
 	int status;
-	atomic_t wait_ctr;
+	unsigned int wait_ctr;
 };
 
 #define AUTOFS_SBI_MAGIC 0x6d4a556d
diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index aa7b526..bcb6c52 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -46,6 +46,7 @@ void autofs4_catatonic_mode(struct autofs_sb_info *sbi)
 			kfree(wq->name.name);
 			wq->name.name = NULL;
 		}
+		wq->wait_ctr--;
 		wake_up_interruptible(&wq->queue);
 		wq = nwq;
 	}
@@ -384,7 +385,7 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
 		wq->pid = current->pid;
 		wq->tgid = current->tgid;
 		wq->status = -EINTR; /* Status return if interrupted */
-		atomic_set(&wq->wait_ctr, 2);
+		wq->wait_ctr = 2;
 		mutex_unlock(&sbi->wq_mutex);
 
 		if (sbi->version < 5) {
@@ -410,7 +411,7 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
 		/* autofs4_notify_daemon() may block */
 		autofs4_notify_daemon(sbi, wq, type);
 	} else {
-		atomic_inc(&wq->wait_ctr);
+		wq->wait_ctr++;
 		mutex_unlock(&sbi->wq_mutex);
 		kfree(qstr.name);
 		DPRINTK("existing wait id = 0x%08lx, name = %.*s, nfy=%d",
@@ -446,8 +447,10 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
 	status = wq->status;
 
 	/* Are we the last process to need status? */
-	if (atomic_dec_and_test(&wq->wait_ctr))
+	mutex_lock(&sbi->wq_mutex);
+	if (!--wq->wait_ctr)
 		kfree(wq);
+	mutex_unlock(&sbi->wq_mutex);
 
 	return status;
 }
@@ -471,14 +474,11 @@ int autofs4_wait_release(struct autofs_sb_info *sbi, autofs_wqt_t wait_queue_tok
 	*wql = wq->next;	/* Unlink from chain */
 	kfree(wq->name.name);
 	wq->name.name = NULL;	/* Do not wait on this queue */
-	mutex_unlock(&sbi->wq_mutex);
-
 	wq->status = status;
-
-	if (atomic_dec_and_test(&wq->wait_ctr))	/* Is anyone still waiting for this guy? */
+	wake_up_interruptible(&wq->queue);
+	if (!--wq->wait_ctr)
 		kfree(wq);
-	else
-		wake_up_interruptible(&wq->queue);
+	mutex_unlock(&sbi->wq_mutex);
 
 	return 0;
 }