linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] autofs updates
@ 2018-11-23 10:41 Ian Kent
  2018-11-23 10:41 ` [PATCH v2 1/5] autofs - improve ioctl sbi checks Ian Kent
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

- improve file system verification for ioctl case.
- fix potential inode leak in mount code.
- add "strictexpire" autofs mount option.

v2 changes:
- fix compile error.
- deffer some of the mount option re-factoring to try and
  avoid any conflicts with linux-next.

---

Ian Kent (5):
      autofs - improve ioctl sbi checks
      autofs - fix possible inode leak in autofs_fill_super()
      autofs - simplify parse_options() function call
      autofs - change catatonic setting to a bit flag
      autofs - add strictexpire mount option


 fs/autofs/autofs_i.h         |   11 ++++--
 fs/autofs/dev-ioctl.c        |   29 +++++-----------
 fs/autofs/init.c             |    2 +
 fs/autofs/inode.c            |   75 +++++++++++++++++++++++++-----------------
 fs/autofs/root.c             |   16 ++++++---
 fs/autofs/waitq.c            |   10 +++---
 include/uapi/linux/auto_fs.h |    2 +
 7 files changed, 78 insertions(+), 67 deletions(-)

--
Ian

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/5] autofs - improve ioctl sbi checks
  2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
@ 2018-11-23 10:41 ` Ian Kent
  2018-11-23 23:29   ` Andrew Morton
  2018-11-23 10:41 ` [PATCH v2 2/5] autofs - fix possible inode leak in autofs_fill_super() Ian Kent
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

Al Viro made some suggestions to improve the implementation
of commit 0633da48f0 "fix autofs_sbi() does not check super
block type".

The check is unnessesary in all cases except for ioctl usage
so placing the check in the super block accessor function
adds a small overhead to the common case where it isn't
needed.

So it's sufficient to do this in the ioctl code only.

Also the check in the ioctl code is needlessly complex.

Signed-off-by: Ian Kent <raven@themaw.net>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
---
 fs/autofs/autofs_i.h  |    3 +--
 fs/autofs/dev-ioctl.c |   25 +++++++------------------
 fs/autofs/init.c      |    2 +-
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 9f9cadbfbd7a..c4301e33c027 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -126,8 +126,7 @@ struct autofs_sb_info {
 
 static inline struct autofs_sb_info *autofs_sbi(struct super_block *sb)
 {
-	return sb->s_magic != AUTOFS_SUPER_MAGIC ?
-		NULL : (struct autofs_sb_info *)(sb->s_fs_info);
+	return (struct autofs_sb_info *)(sb->s_fs_info);
 }
 
 static inline struct autofs_info *autofs_dentry_ino(struct dentry *dentry)
diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c
index 86eafda4a652..3de4f5e61caf 100644
--- a/fs/autofs/dev-ioctl.c
+++ b/fs/autofs/dev-ioctl.c
@@ -14,6 +14,8 @@
 
 #include "autofs_i.h"
 
+extern struct file_system_type autofs_fs_type;
+
 /*
  * This module implements an interface for routing autofs ioctl control
  * commands via a miscellaneous device file.
@@ -151,22 +153,6 @@ static int validate_dev_ioctl(int cmd, struct autofs_dev_ioctl *param)
 	return err;
 }
 
-/*
- * Get the autofs super block info struct from the file opened on
- * the autofs mount point.
- */
-static struct autofs_sb_info *autofs_dev_ioctl_sbi(struct file *f)
-{
-	struct autofs_sb_info *sbi = NULL;
-	struct inode *inode;
-
-	if (f) {
-		inode = file_inode(f);
-		sbi = autofs_sbi(inode->i_sb);
-	}
-	return sbi;
-}
-
 /* Return autofs dev ioctl version */
 static int autofs_dev_ioctl_version(struct file *fp,
 				    struct autofs_sb_info *sbi,
@@ -658,6 +644,8 @@ static int _autofs_dev_ioctl(unsigned int command,
 	if (cmd != AUTOFS_DEV_IOCTL_VERSION_CMD &&
 	    cmd != AUTOFS_DEV_IOCTL_OPENMOUNT_CMD &&
 	    cmd != AUTOFS_DEV_IOCTL_CLOSEMOUNT_CMD) {
+		struct super_block *sb;
+
 		fp = fget(param->ioctlfd);
 		if (!fp) {
 			if (cmd == AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD)
@@ -666,12 +654,13 @@ static int _autofs_dev_ioctl(unsigned int command,
 			goto out;
 		}
 
-		sbi = autofs_dev_ioctl_sbi(fp);
-		if (!sbi || sbi->magic != AUTOFS_SBI_MAGIC) {
+		sb = file_inode(fp)->i_sb;
+		if (sb->s_type != &autofs_fs_type) {
 			err = -EINVAL;
 			fput(fp);
 			goto out;
 		}
+		sbi = autofs_sbi(sb);
 
 		/*
 		 * Admin needs to be able to set the mount catatonic in
diff --git a/fs/autofs/init.c b/fs/autofs/init.c
index 79ae07d9592f..c0c1db2cc6ea 100644
--- a/fs/autofs/init.c
+++ b/fs/autofs/init.c
@@ -16,7 +16,7 @@ static struct dentry *autofs_mount(struct file_system_type *fs_type,
 	return mount_nodev(fs_type, flags, data, autofs_fill_super);
 }
 
-static struct file_system_type autofs_fs_type = {
+struct file_system_type autofs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "autofs",
 	.mount		= autofs_mount,

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/5] autofs - fix possible inode leak in autofs_fill_super()
  2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
  2018-11-23 10:41 ` [PATCH v2 1/5] autofs - improve ioctl sbi checks Ian Kent
@ 2018-11-23 10:41 ` Ian Kent
  2018-11-23 10:42 ` [PATCH v2 3/5] autofs - simplify parse_options() function call Ian Kent
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:41 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

There is no check at all for a failure to allocate the root inode
in autofs_fill_super(), handle it.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 846c052569dd..e5c06b5a7371 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -254,9 +254,13 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 		goto fail_free;
 	}
 	root_inode = autofs_get_inode(s, S_IFDIR | 0755);
+	if (!root_inode) {
+		ret = -ENOMEM;
+		goto fail_ino;
+	}
 	root = d_make_root(root_inode);
 	if (!root)
-		goto fail_ino;
+		goto fail_iput;
 	pipe = NULL;
 
 	root->d_fsdata = ino;
@@ -304,8 +308,8 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	root_inode->i_op = &autofs_dir_inode_operations;
 
 	pr_debug("pipe fd = %d, pgrp = %u\n", pipefd, pid_nr(sbi->oz_pgrp));
-	pipe = fget(pipefd);
 
+	pipe = fget(pipefd);
 	if (!pipe) {
 		pr_err("could not open pipe file descriptor\n");
 		goto fail_put_pid;
@@ -334,6 +338,8 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 fail_dput:
 	dput(root);
 	goto fail_free;
+fail_iput:
+	iput(root_inode);
 fail_ino:
 	autofs_free_ino(ino);
 fail_free:

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/5] autofs - simplify parse_options() function call
  2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
  2018-11-23 10:41 ` [PATCH v2 1/5] autofs - improve ioctl sbi checks Ian Kent
  2018-11-23 10:41 ` [PATCH v2 2/5] autofs - fix possible inode leak in autofs_fill_super() Ian Kent
@ 2018-11-23 10:42 ` Ian Kent
  2018-11-23 10:42 ` [PATCH v2 4/5] autofs - change catatonic setting to a bit flag Ian Kent
  2018-11-23 10:42 ` [PATCH v2 5/5] autofs - add strictexpire mount option Ian Kent
  4 siblings, 0 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

The parse_options() function uses a long list of parameters, most
of which are present in the super block info structure already.

The mount parameters set in parse_options() options don't require
cleanup so using the super block info struct directly is simpler.
---
 fs/autofs/inode.c |   55 ++++++++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 26 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index e5c06b5a7371..ccedd12c1ac5 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -124,21 +124,24 @@ static const match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
-static int parse_options(char *options, int *pipefd, kuid_t *uid, kgid_t *gid,
-			 int *pgrp, bool *pgrp_set, unsigned int *type,
-			 int *minproto, int *maxproto)
+static int parse_options(char *options,
+			 struct inode *root, int *pgrp, bool *pgrp_set,
+			 struct autofs_sb_info *sbi)
 {
 	char *p;
 	substring_t args[MAX_OPT_ARGS];
 	int option;
+	int pipefd = -1;
+	kuid_t uid;
+	kgid_t gid;
 
-	*uid = current_uid();
-	*gid = current_gid();
+	root->i_uid = current_uid();
+	root->i_gid = current_gid();
 
-	*minproto = AUTOFS_MIN_PROTO_VERSION;
-	*maxproto = AUTOFS_MAX_PROTO_VERSION;
+	sbi->min_proto = AUTOFS_MIN_PROTO_VERSION;
+	sbi->max_proto = AUTOFS_MAX_PROTO_VERSION;
 
-	*pipefd = -1;
+	sbi->pipefd = -1;
 
 	if (!options)
 		return 1;
@@ -152,22 +155,25 @@ static int parse_options(char *options, int *pipefd, kuid_t *uid, kgid_t *gid,
 		token = match_token(p, tokens, args);
 		switch (token) {
 		case Opt_fd:
-			if (match_int(args, pipefd))
+			if (match_int(args, &pipefd))
 				return 1;
+			sbi->pipefd = pipefd;
 			break;
 		case Opt_uid:
 			if (match_int(args, &option))
 				return 1;
-			*uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(*uid))
+			uid = make_kuid(current_user_ns(), option);
+			if (!uid_valid(uid))
 				return 1;
+			root->i_uid = uid;
 			break;
 		case Opt_gid:
 			if (match_int(args, &option))
 				return 1;
-			*gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(*gid))
+			gid = make_kgid(current_user_ns(), option);
+			if (!gid_valid(gid))
 				return 1;
+			root->i_gid = gid;
 			break;
 		case Opt_pgrp:
 			if (match_int(args, &option))
@@ -178,27 +184,27 @@ static int parse_options(char *options, int *pipefd, kuid_t *uid, kgid_t *gid,
 		case Opt_minproto:
 			if (match_int(args, &option))
 				return 1;
-			*minproto = option;
+			sbi->min_proto = option;
 			break;
 		case Opt_maxproto:
 			if (match_int(args, &option))
 				return 1;
-			*maxproto = option;
+			sbi->max_proto = option;
 			break;
 		case Opt_indirect:
-			set_autofs_type_indirect(type);
+			set_autofs_type_indirect(&sbi->type);
 			break;
 		case Opt_direct:
-			set_autofs_type_direct(type);
+			set_autofs_type_direct(&sbi->type);
 			break;
 		case Opt_offset:
-			set_autofs_type_offset(type);
+			set_autofs_type_offset(&sbi->type);
 			break;
 		default:
 			return 1;
 		}
 	}
-	return (*pipefd < 0);
+	return (sbi->pipefd < 0);
 }
 
 int autofs_fill_super(struct super_block *s, void *data, int silent)
@@ -206,7 +212,6 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	struct inode *root_inode;
 	struct dentry *root;
 	struct file *pipe;
-	int pipefd;
 	struct autofs_sb_info *sbi;
 	struct autofs_info *ino;
 	int pgrp = 0;
@@ -266,9 +271,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	root->d_fsdata = ino;
 
 	/* Can this call block? */
-	if (parse_options(data, &pipefd, &root_inode->i_uid, &root_inode->i_gid,
-			  &pgrp, &pgrp_set, &sbi->type, &sbi->min_proto,
-			  &sbi->max_proto)) {
+	if (parse_options(data, root_inode, &pgrp, &pgrp_set, sbi)) {
 		pr_err("called with bogus options\n");
 		goto fail_dput;
 	}
@@ -307,9 +310,10 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	root_inode->i_fop = &autofs_root_operations;
 	root_inode->i_op = &autofs_dir_inode_operations;
 
-	pr_debug("pipe fd = %d, pgrp = %u\n", pipefd, pid_nr(sbi->oz_pgrp));
+	pr_debug("pipe fd = %d, pgrp = %u\n",
+		 sbi->pipefd, pid_nr(sbi->oz_pgrp));
 
-	pipe = fget(pipefd);
+	pipe = fget(sbi->pipefd);
 	if (!pipe) {
 		pr_err("could not open pipe file descriptor\n");
 		goto fail_put_pid;
@@ -318,7 +322,6 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	if (ret < 0)
 		goto fail_fput;
 	sbi->pipe = pipe;
-	sbi->pipefd = pipefd;
 	sbi->catatonic = 0;
 
 	/*

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 4/5] autofs - change catatonic setting to a bit flag
  2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
                   ` (2 preceding siblings ...)
  2018-11-23 10:42 ` [PATCH v2 3/5] autofs - simplify parse_options() function call Ian Kent
@ 2018-11-23 10:42 ` Ian Kent
  2018-11-23 10:42 ` [PATCH v2 5/5] autofs - add strictexpire mount option Ian Kent
  4 siblings, 0 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

Change the super block info. catatonic setting to be part of a
flags bit field.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/autofs_i.h  |    7 +++++--
 fs/autofs/dev-ioctl.c |    4 ++--
 fs/autofs/inode.c     |    4 ++--
 fs/autofs/root.c      |   11 ++++++-----
 fs/autofs/waitq.c     |   10 +++++-----
 5 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index c4301e33c027..032cbb12531a 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -101,16 +101,18 @@ struct autofs_wait_queue {
 
 #define AUTOFS_SBI_MAGIC 0x6d4a556d
 
+#define AUTOFS_SBI_CATATONIC	0x0001
+
 struct autofs_sb_info {
 	u32 magic;
 	int pipefd;
 	struct file *pipe;
 	struct pid *oz_pgrp;
-	int catatonic;
 	int version;
 	int sub_version;
 	int min_proto;
 	int max_proto;
+	unsigned int flags;
 	unsigned long exp_timeout;
 	unsigned int type;
 	struct super_block *sb;
@@ -140,7 +142,8 @@ static inline struct autofs_info *autofs_dentry_ino(struct dentry *dentry)
  */
 static inline int autofs_oz_mode(struct autofs_sb_info *sbi)
 {
-	return sbi->catatonic || task_pgrp(current) == sbi->oz_pgrp;
+	return ((sbi->flags & AUTOFS_SBI_CATATONIC) ||
+		 task_pgrp(current) == sbi->oz_pgrp);
 }
 
 struct inode *autofs_get_inode(struct super_block *, umode_t);
diff --git a/fs/autofs/dev-ioctl.c b/fs/autofs/dev-ioctl.c
index 3de4f5e61caf..fc1caf449fa3 100644
--- a/fs/autofs/dev-ioctl.c
+++ b/fs/autofs/dev-ioctl.c
@@ -352,7 +352,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
 	pipefd = param->setpipefd.pipefd;
 
 	mutex_lock(&sbi->wq_mutex);
-	if (!sbi->catatonic) {
+	if (!(sbi->flags & AUTOFS_SBI_CATATONIC)) {
 		mutex_unlock(&sbi->wq_mutex);
 		return -EBUSY;
 	} else {
@@ -379,7 +379,7 @@ static int autofs_dev_ioctl_setpipefd(struct file *fp,
 		swap(sbi->oz_pgrp, new_pid);
 		sbi->pipefd = pipefd;
 		sbi->pipe = pipe;
-		sbi->catatonic = 0;
+		sbi->flags &= ~AUTOFS_SBI_CATATONIC;
 	}
 out:
 	put_pid(new_pid);
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index ccedd12c1ac5..8cfd3f67af57 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -227,12 +227,12 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	sbi->magic = AUTOFS_SBI_MAGIC;
 	sbi->pipefd = -1;
 	sbi->pipe = NULL;
-	sbi->catatonic = 1;
 	sbi->exp_timeout = 0;
 	sbi->oz_pgrp = NULL;
 	sbi->sb = s;
 	sbi->version = 0;
 	sbi->sub_version = 0;
+	sbi->flags = AUTOFS_SBI_CATATONIC;
 	set_autofs_type_indirect(&sbi->type);
 	sbi->min_proto = 0;
 	sbi->max_proto = 0;
@@ -322,7 +322,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	if (ret < 0)
 		goto fail_fput;
 	sbi->pipe = pipe;
-	sbi->catatonic = 0;
+	sbi->flags &= ~AUTOFS_SBI_CATATONIC;
 
 	/*
 	 * Success! Install the root dentry now to indicate completion.
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 782e57b911ab..164ccd3402cf 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -510,7 +510,8 @@ static struct dentry *autofs_lookup(struct inode *dir,
 	sbi = autofs_sbi(dir->i_sb);
 
 	pr_debug("pid = %u, pgrp = %u, catatonic = %d, oz_mode = %d\n",
-		 current->pid, task_pgrp_nr(current), sbi->catatonic,
+		 current->pid, task_pgrp_nr(current),
+		 sbi->flags & AUTOFS_SBI_CATATONIC,
 		 autofs_oz_mode(sbi));
 
 	active = autofs_lookup_active(dentry);
@@ -563,7 +564,7 @@ static int autofs_dir_symlink(struct inode *dir,
 	 * autofs mount is catatonic but the state of an autofs
 	 * file system needs to be preserved over restarts.
 	 */
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -EACCES;
 
 	BUG_ON(!ino);
@@ -626,7 +627,7 @@ static int autofs_dir_unlink(struct inode *dir, struct dentry *dentry)
 	 * autofs mount is catatonic but the state of an autofs
 	 * file system needs to be preserved over restarts.
 	 */
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -EACCES;
 
 	if (atomic_dec_and_test(&ino->count)) {
@@ -714,7 +715,7 @@ static int autofs_dir_rmdir(struct inode *dir, struct dentry *dentry)
 	 * autofs mount is catatonic but the state of an autofs
 	 * file system needs to be preserved over restarts.
 	 */
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -EACCES;
 
 	spin_lock(&sbi->lookup_lock);
@@ -759,7 +760,7 @@ static int autofs_dir_mkdir(struct inode *dir,
 	 * autofs mount is catatonic but the state of an autofs
 	 * file system needs to be preserved over restarts.
 	 */
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -EACCES;
 
 	pr_debug("dentry %p, creating %pd\n", dentry, dentry);
diff --git a/fs/autofs/waitq.c b/fs/autofs/waitq.c
index f6385c6ef0a5..15a3e31d0904 100644
--- a/fs/autofs/waitq.c
+++ b/fs/autofs/waitq.c
@@ -20,14 +20,14 @@ void autofs_catatonic_mode(struct autofs_sb_info *sbi)
 	struct autofs_wait_queue *wq, *nwq;
 
 	mutex_lock(&sbi->wq_mutex);
-	if (sbi->catatonic) {
+	if (sbi->flags & AUTOFS_SBI_CATATONIC) {
 		mutex_unlock(&sbi->wq_mutex);
 		return;
 	}
 
 	pr_debug("entering catatonic mode\n");
 
-	sbi->catatonic = 1;
+	sbi->flags |= AUTOFS_SBI_CATATONIC;
 	wq = sbi->queues;
 	sbi->queues = NULL;	/* Erase all wait queues */
 	while (wq) {
@@ -255,7 +255,7 @@ static int validate_request(struct autofs_wait_queue **wait,
 	struct autofs_wait_queue *wq;
 	struct autofs_info *ino;
 
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -ENOENT;
 
 	/* Wait in progress, continue; */
@@ -290,7 +290,7 @@ static int validate_request(struct autofs_wait_queue **wait,
 			if (mutex_lock_interruptible(&sbi->wq_mutex))
 				return -EINTR;
 
-			if (sbi->catatonic)
+			if (sbi->flags & AUTOFS_SBI_CATATONIC)
 				return -ENOENT;
 
 			wq = autofs_find_wait(sbi, qstr);
@@ -359,7 +359,7 @@ int autofs_wait(struct autofs_sb_info *sbi,
 	pid_t tgid;
 
 	/* In catatonic mode, we don't wait for nobody */
-	if (sbi->catatonic)
+	if (sbi->flags & AUTOFS_SBI_CATATONIC)
 		return -ENOENT;
 
 	/*

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 5/5] autofs - add strictexpire mount option
  2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
                   ` (3 preceding siblings ...)
  2018-11-23 10:42 ` [PATCH v2 4/5] autofs - change catatonic setting to a bit flag Ian Kent
@ 2018-11-23 10:42 ` Ian Kent
  4 siblings, 0 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-23 10:42 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

Commit 092a53452b (("autofs: take more care to not update last_used
on path walk") helped to (partially) resolve a problem where automounts
were not expiring due to aggressive accesses from user space.

This patch was later reverted because, for very large environments,
it meant more mount requests from clients and when there are a lot
of clients this caused a fairly significant increase in server load.

But there is a need for both types of expire check, depending on use
case, so add a mount option to allow for strict update of last use
of autofs dentrys (which just means not updating the last use on path
walk access).

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/autofs_i.h         |    1 +
 fs/autofs/inode.c            |    8 +++++++-
 fs/autofs/root.c             |    5 ++++-
 include/uapi/linux/auto_fs.h |    2 +-
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 032cbb12531a..fa3733beb52e 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -102,6 +102,7 @@ struct autofs_wait_queue {
 #define AUTOFS_SBI_MAGIC 0x6d4a556d
 
 #define AUTOFS_SBI_CATATONIC	0x0001
+#define AUTOFS_SBI_STRICTEXPIRE 0x0002
 
 struct autofs_sb_info {
 	u32 magic;
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 8cfd3f67af57..501833cc49a8 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -87,6 +87,8 @@ static int autofs_show_options(struct seq_file *m, struct dentry *root)
 		seq_printf(m, ",direct");
 	else
 		seq_printf(m, ",indirect");
+	if (sbi->flags & AUTOFS_SBI_STRICTEXPIRE)
+		seq_printf(m, ",strictexpire");
 #ifdef CONFIG_CHECKPOINT_RESTORE
 	if (sbi->pipe)
 		seq_printf(m, ",pipe_ino=%ld", file_inode(sbi->pipe)->i_ino);
@@ -109,7 +111,7 @@ static const struct super_operations autofs_sops = {
 };
 
 enum {Opt_err, Opt_fd, Opt_uid, Opt_gid, Opt_pgrp, Opt_minproto, Opt_maxproto,
-	Opt_indirect, Opt_direct, Opt_offset};
+	Opt_indirect, Opt_direct, Opt_offset, Opt_strictexpire};
 
 static const match_table_t tokens = {
 	{Opt_fd, "fd=%u"},
@@ -121,6 +123,7 @@ static const match_table_t tokens = {
 	{Opt_indirect, "indirect"},
 	{Opt_direct, "direct"},
 	{Opt_offset, "offset"},
+	{Opt_strictexpire, "strictexpire"},
 	{Opt_err, NULL}
 };
 
@@ -200,6 +203,9 @@ static int parse_options(char *options,
 		case Opt_offset:
 			set_autofs_type_offset(&sbi->type);
 			break;
+		case Opt_strictexpire:
+			sbi->flags |= AUTOFS_SBI_STRICTEXPIRE;
+			break;
 		default:
 			return 1;
 		}
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index 164ccd3402cf..1246f396bf0e 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -275,8 +275,11 @@ static int autofs_mount_wait(const struct path *path, bool rcu_walk)
 		pr_debug("waiting for mount name=%pd\n", path->dentry);
 		status = autofs_wait(sbi, path, NFY_MOUNT);
 		pr_debug("mount wait done status=%d\n", status);
+		ino->last_used = jiffies;
+		return status;
 	}
-	ino->last_used = jiffies;
+	if (!(sbi->flags & AUTOFS_SBI_STRICTEXPIRE))
+		ino->last_used = jiffies;
 	return status;
 }
 
diff --git a/include/uapi/linux/auto_fs.h b/include/uapi/linux/auto_fs.h
index df31aa9c9a8c..082119630b49 100644
--- a/include/uapi/linux/auto_fs.h
+++ b/include/uapi/linux/auto_fs.h
@@ -23,7 +23,7 @@
 #define AUTOFS_MIN_PROTO_VERSION	3
 #define AUTOFS_MAX_PROTO_VERSION	5
 
-#define AUTOFS_PROTO_SUBVERSION		3
+#define AUTOFS_PROTO_SUBVERSION		4
 
 /*
  * The wait_queue_token (autofs_wqt_t) is part of a structure which is passed

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/5] autofs - improve ioctl sbi checks
  2018-11-23 10:41 ` [PATCH v2 1/5] autofs - improve ioctl sbi checks Ian Kent
@ 2018-11-23 23:29   ` Andrew Morton
  2018-11-25 22:57     ` Ian Kent
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2018-11-23 23:29 UTC (permalink / raw)
  To: Ian Kent; +Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

On Fri, 23 Nov 2018 18:41:50 +0800 Ian Kent <raven@themaw.net> wrote:

> Al Viro made some suggestions to improve the implementation
> of commit 0633da48f0 "fix autofs_sbi() does not check super
> block type".
> 
> The check is unnessesary in all cases except for ioctl usage
> so placing the check in the super block accessor function
> adds a small overhead to the common case where it isn't
> needed.
> 
> So it's sufficient to do this in the ioctl code only.
> 
> Also the check in the ioctl code is needlessly complex.
>
> ...
>
> --- a/fs/autofs/dev-ioctl.c
> +++ b/fs/autofs/dev-ioctl.c
> @@ -14,6 +14,8 @@
>  
>  #include "autofs_i.h"
>  
> +extern struct file_system_type autofs_fs_type;
> +
>  /*
>   * This module implements an interface for routing autofs ioctl control
>   * commands via a miscellaneous device file.

It's naughty to declare externs in C files, for various reasons.  Is
this OK?

--- a/fs/autofs/autofs_i.h~autofs-improve-ioctl-sbi-checks-fix
+++ a/fs/autofs/autofs_i.h
@@ -42,6 +42,8 @@
 #endif
 #define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__
 
+extern struct file_system_type autofs_fs_type;
+
 /*
  * Unified info structure.  This is pointed to by both the dentry and
  * inode structures.  Each file in the filesystem has an instance of this
--- a/fs/autofs/dev-ioctl.c~autofs-improve-ioctl-sbi-checks-fix
+++ a/fs/autofs/dev-ioctl.c
@@ -14,8 +14,6 @@
 
 #include "autofs_i.h"
 
-extern struct file_system_type autofs_fs_type;
-
 /*
  * This module implements an interface for routing autofs ioctl control
  * commands via a miscellaneous device file.
_

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/5] autofs - improve ioctl sbi checks
  2018-11-23 23:29   ` Andrew Morton
@ 2018-11-25 22:57     ` Ian Kent
  0 siblings, 0 replies; 8+ messages in thread
From: Ian Kent @ 2018-11-25 22:57 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Al Viro, autofs mailing list, linux-fsdevel, Kernel Mailing List

On Fri, 2018-11-23 at 15:29 -0800, Andrew Morton wrote:
> On Fri, 23 Nov 2018 18:41:50 +0800 Ian Kent <raven@themaw.net> wrote:
> 
> > Al Viro made some suggestions to improve the implementation
> > of commit 0633da48f0 "fix autofs_sbi() does not check super
> > block type".
> > 
> > The check is unnessesary in all cases except for ioctl usage
> > so placing the check in the super block accessor function
> > adds a small overhead to the common case where it isn't
> > needed.
> > 
> > So it's sufficient to do this in the ioctl code only.
> > 
> > Also the check in the ioctl code is needlessly complex.
> > 
> > ...
> > 
> > --- a/fs/autofs/dev-ioctl.c
> > +++ b/fs/autofs/dev-ioctl.c
> > @@ -14,6 +14,8 @@
> >  
> >  #include "autofs_i.h"
> >  
> > +extern struct file_system_type autofs_fs_type;
> > +
> >  /*
> >   * This module implements an interface for routing autofs ioctl control
> >   * commands via a miscellaneous device file.
> 
> It's naughty to declare externs in C files, for various reasons.  Is
> this OK?

OK, I understand the reasoning I guess.

The change below is fine.

Thanks, Ian.

> 
> --- a/fs/autofs/autofs_i.h~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/autofs_i.h
> @@ -42,6 +42,8 @@
>  #endif
>  #define pr_fmt(fmt) KBUILD_MODNAME ":pid:%d:%s: " fmt, current->pid, __func__
>  
> +extern struct file_system_type autofs_fs_type;
> +
>  /*
>   * Unified info structure.  This is pointed to by both the dentry and
>   * inode structures.  Each file in the filesystem has an instance of this
> --- a/fs/autofs/dev-ioctl.c~autofs-improve-ioctl-sbi-checks-fix
> +++ a/fs/autofs/dev-ioctl.c
> @@ -14,8 +14,6 @@
>  
>  #include "autofs_i.h"
>  
> -extern struct file_system_type autofs_fs_type;
> -
>  /*
>   * This module implements an interface for routing autofs ioctl control
>   * commands via a miscellaneous device file.
> _
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-11-26  9:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-23 10:41 [PATCH v2 0/5] autofs updates Ian Kent
2018-11-23 10:41 ` [PATCH v2 1/5] autofs - improve ioctl sbi checks Ian Kent
2018-11-23 23:29   ` Andrew Morton
2018-11-25 22:57     ` Ian Kent
2018-11-23 10:41 ` [PATCH v2 2/5] autofs - fix possible inode leak in autofs_fill_super() Ian Kent
2018-11-23 10:42 ` [PATCH v2 3/5] autofs - simplify parse_options() function call Ian Kent
2018-11-23 10:42 ` [PATCH v2 4/5] autofs - change catatonic setting to a bit flag Ian Kent
2018-11-23 10:42 ` [PATCH v2 5/5] autofs - add strictexpire mount option Ian Kent

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).