LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ian Kent <raven@themaw.net>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Kernel Mailing List <linux-kernel@vger.kernel.org>,
	autofs mailing list <autofs@linux.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH 4/4] autofs4 - add miscelaneous device for ioctls
Date: Thu, 28 Feb 2008 15:18:59 +0900	[thread overview]
Message-ID: <1204179539.3501.17.camel@raven.themaw.net> (raw)
In-Reply-To: <20080227211723.d5d27c09.akpm@linux-foundation.org>


On Wed, 2008-02-27 at 21:17 -0800, Andrew Morton wrote:
> On Tue, 26 Feb 2008 12:23:55 +0900 (WST) Ian Kent <raven@themaw.net> wrote:
> 
> > Hi Andrew,
> > 
> > Patch to add miscellaneous device to autofs4 module for
> > ioctls.
> 
> Could you please document the new kernel interface which you're proposing? 
> In Docmentation/ or in the changelog?

Also a good suggestion.
I'll include that too.

> 
> We seem to be passing some string into a miscdevice ioctl and getting some
> results back.  Be aware that this won't be a terribly popular proposal, so
> I'd suggest that you fully describe the problem which it's trying to solve,
> and how it solves it, and why the various alternatives (sysfs, netlink,
> mount options, etc) were judged unsuitable.

Yes, as I said above.

I don't expect that people that aren't close to the development of
autofs will "get" the problem description in the leading post but I will
try and expand on it as best I can.

As for the possible alternatives, it sounds like I have some more work
to do on that. Mount options can't be used as I described in the lead in
post and, as far as my understanding of sysfs goes, I don't think it's
appropriate. But, I'm not aware of what the netlink interface may be
able to do for me so I will need to check on that.

> 
> 
> > ...
> > 
> > --- linux-2.6.25-rc2-mm1/fs/autofs4/init.c.device-node-ioctl	2008-01-25 07:58:37.000000000 +0900
> > +++ linux-2.6.25-rc2-mm1/fs/autofs4/init.c	2008-02-22 11:51:41.000000000 +0900
> > @@ -29,11 +29,20 @@ static struct file_system_type autofs_fs
> >  
> >  static int __init init_autofs4_fs(void)
> >  {
> > -	return register_filesystem(&autofs_fs_type);
> > +	int err;
> > +
> > +	err = register_filesystem(&autofs_fs_type);
> > +	if (err)
> > +		return err;
> > +
> > +	err = autofs_dev_ioctl_init();
> > +
> > +	return err;
> >  }
> 
> We should run unregister_filesystem() if autofs_dev_ioctl_init() fails.
> 
> >  static void __exit exit_autofs4_fs(void)
> >  {
> > +	autofs_dev_ioctl_exit();
> >  	unregister_filesystem(&autofs_fs_type);
> >  }
> >  
> > diff -up linux-2.6.25-rc2-mm1/fs/autofs4/autofs_i.h.device-node-ioctl linux-2.6.25-rc2-mm1/fs/autofs4/autofs_i.h
> > --- linux-2.6.25-rc2-mm1/fs/autofs4/autofs_i.h.device-node-ioctl	2008-02-22 11:51:41.000000000 +0900
> > +++ linux-2.6.25-rc2-mm1/fs/autofs4/autofs_i.h	2008-02-22 11:51:41.000000000 +0900
> > @@ -14,6 +14,7 @@
> >  /* Internal header file for autofs */
> >  
> >  #include <linux/auto_fs4.h>
> > +#include <linux/auto_dev-ioctl.h>
> >  #include <linux/mutex.h>
> >  #include <linux/list.h>
> >  
> > @@ -40,6 +41,16 @@
> >  #define DPRINTK(fmt,args...) do {} while(0)
> >  #endif
> >  
> > +#define WARN(fmt,args...) \
> > +do {			  \
> > +	printk("KERN_WARNING pid %d: %s: " fmt "\n" , current->pid , __FUNCTION__ , ##args); \
> > +} while(0)
> > +
> > +#define ERROR(fmt,args...) \
> > +do {			  \
> > +	printk("KERN_ERR pid %d: %s: " fmt "\n" , current->pid , __FUNCTION__ , ##args); \
> > +} while(0)
> 
> Please always pass all diffs through scripts/checkpatch.pl.
> 
> >  /* Unified info structure.  This is pointed to by both the dentry and
> >     inode structures.  Each file in the filesystem has an instance of this
> >     structure.  It holds a reference to the dentry, so dentries are never
> > @@ -172,6 +183,17 @@ int autofs4_expire_run(struct super_bloc
> >  			struct autofs_packet_expire __user *);
> >  int autofs4_expire_multi(struct super_block *, struct vfsmount *,
> >  			struct autofs_sb_info *, int __user *);
> > +struct dentry *autofs4_expire_direct(struct super_block *sb,
> > +				     struct vfsmount *mnt,
> > +				     struct autofs_sb_info *sbi, int how);
> > +struct dentry *autofs4_expire_indirect(struct super_block *sb,
> > +				       struct vfsmount *mnt,
> > +				       struct autofs_sb_info *sbi, int how);
> > +
> > +/* Device node initialization */
> > +
> > +int autofs_dev_ioctl_init(void);
> > +void autofs_dev_ioctl_exit(void);
> >  
> > ...
> >
> > @@ -0,0 +1,788 @@
> > +/*
> > + * linux/fs/autofs4/dev-ioctl.c
> 
> We prefer not to bother with the filename-in-the-file thing.  You know what
> file you're reading, and these things tend to not get updated across
> renames.
> 
> > + * Copyright 2008 Red Hat, Inc. All rights reserved.
> > + * Copyright 2008 Ian Kent <raven@themaw.net>
> > + *
> > + * This file is part of the Linux kernel and is made available under
> > + * the terms of the GNU General Public License, version 2, or at your
> > + * option, any later version, incorporated herein by reference.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/vmalloc.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/init.h>
> > +#include <linux/wait.h>
> > +#include <linux/namei.h>
> > +#include <linux/fcntl.h>
> > +#include <linux/file.h>
> > +#include <linux/sched.h>
> > +#include <linux/compat.h>
> > +#include <linux/syscalls.h>
> > +#include <linux/smp_lock.h>
> > +#include <linux/magic.h>
> > +#include <linux/dcache.h>
> > +#include <asm/uaccess.h>
> 
> Please include <linux/foo.h> rather than <asm/foo.h> if the former exists.
> 
> > +#include "autofs_i.h"
> > +
> > +/*
> > + * This module implements an interface for routing autofs ioctl control
> > + * commands via a miscellaneous device file.
> > + *
> > + * The alternate interface is needed because we need to be able open
> > + * an ioctl file descriptor on an autofs mount that may be covered by
> > + * another mount. This situation arises when starting automount(8)
> > + * or other user space daemon which uses direct mounts or offset
> > + * mounts (used for autofs lazy mount/umount of nested mount trees),
> > + * which have been left busy at at service shutdown.
> > + */
> > +
> > +#define AUTOFS_DEVICE_NAME	"autofs"
> > +
> > +#define AUTOFS_DEV_IOCTL_IOC_FIRST AUTOFS_DEV_IOCTL_VERSION
> > +#define AUTOFS_DEV_IOCTL_IOC_COUNT AUTOFS_IOC_COUNT - 11
> 
> This needs parentheses.
> 
> Shouldn't these be in a header file, exported to userspace builds?
> 
> > +#define AUTOFS_DEV_IOCTL_SIZE	sizeof(struct autofs_dev_ioctl)
> > +
> > +typedef int (*ioctl_fn)(struct file *, struct autofs_sb_info *, struct autofs_dev_ioctl *);
> > +
> > +static int check_name(const char *name)
> > +{
> > +	if (!strchr(name, '/'))
> > +		return -EINVAL;
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Check a string doesn't overrun the chunk of
> > + * memory we copied from user land.
> > + */
> > +static int invalid_str(char *str, void *end)
> > +{
> > +	while ((void *) str <= end)
> > +		if (!*str++)
> > +			return 0;
> > +	return -EINVAL;
> > +}
> >
> > ...
> >
> > +/*
> > + * 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) {
> 
> `f' cannot be NULL here.
> 
> > +		inode = f->f_path.dentry->d_inode;
> > +		sbi = autofs4_sbi(inode->i_sb);
> > +	}
> > +
> > +	return sbi;
> > +}
> > +
> > +/* Return autofs module protocol version */
> > +static inline int autofs_dev_ioctl_protover(struct file *fp,
> > +					    struct autofs_sb_info *sbi,
> > +					    struct autofs_dev_ioctl *param)
> > +{
> > +	param->arg1 = sbi->version;
> > +	return 0;
> > +}
> > +
> > +/* Return autofs module protocol sub version */
> > +static inline int autofs_dev_ioctl_protosubver(struct file *fp,
> > +					       struct autofs_sb_info *sbi,
> > +					       struct autofs_dev_ioctl *param)
> > +{
> > +	param->arg1 = sbi->sub_version;
> > +	return 0;
> > +}
> 
> Don't bother inlining things - the compiler will do it.
> 
> > +/*
> > + * Walk down the mount stack looking for an autofs mount that
> > + * has the requested device number (aka. new_encode_dev(sb->s_dev).
> > + */
> > +static int autofs_dev_ioctl_find_super(struct nameidata *nd, dev_t devno)
> > +{
> > +	struct dentry *dentry;
> > +	struct inode *inode;
> > +	struct super_block *sb;
> > +	dev_t s_dev;
> > +	unsigned int err;
> > +
> > +	err = -ENOENT;
> > +
> > +	/* Lookup the dentry name at the base of our mount point */
> > +	dentry = d_lookup(nd->path.dentry, &nd->last);
> > +	if (!dentry)
> > +		goto out;
> > +
> > +	dput(nd->path.dentry);
> > +	nd->path.dentry = dentry;
> > +
> > +	/* And follow the mount stack looking for our autofs mount */
> > +	while (1) {
> > +		inode = nd->path.dentry->d_inode;
> > +		if (!inode)
> > +			continue;
> > +
> > +		sb = inode->i_sb;
> > +		s_dev = new_encode_dev(sb->s_dev);
> > +		if (devno == s_dev) {
> > +			if (sb->s_magic == AUTOFS_SUPER_MAGIC) {
> > +				err = 0;
> > +				break;
> > +			}
> > +		}
> > +
> > +		if (!follow_down(&nd->path.mnt, &nd->path.dentry))
> > +			goto out;
> > +	}
> > +
> > +out:
> > +	return err;
> > +}
> 
> hm.  possibly-interested parties cc'ed.
> 
> > +/*
> > + * Install the file opened for autofs mount point control functions
> > + * and set close on exec.
> > + */
> > +static void autofs_dev_ioctl_fd_install(unsigned int fd, struct file *file)
> > +{
> > +	struct files_struct *files = current->files;
> > +	struct fdtable *fdt;
> > +
> > +	spin_lock(&files->file_lock);
> > +	fdt = files_fdtable(files);
> > +	BUG_ON(fdt->fd[fd] != NULL);
> > +	rcu_assign_pointer(fdt->fd[fd], file);
> > +	FD_SET(fd, fdt->close_on_exec);
> > +	spin_unlock(&files->file_lock);
> > +}
> 
> That's fd_install() plus an add-on.  It's not autofs-specific.  Should be
> in fs/open.c, methinks?
> 
> >
> > ...
> >
> > +/* Close file descriptor allocated above (user can also use close(2)). */
> > +static inline int autofs_dev_ioctl_closemount(struct file *fp,
> > +					      struct autofs_sb_info *sbi,
> > +					      struct autofs_dev_ioctl *param)
> > +{
> > +	return sys_close(param->ioctlfd);
> > +}
> 
> hm.
> 
> >
> > ...
> >
> > +/*
> > + * Set the pipe fd for kernel communication to the daemon.
> > + *
> > + * Normally this is set at mount using an option but if we
> > + * are reconnecting to a busy mount then we need to use this
> > + * to tell the autofs mount about the new kernel pipe fd. In
> > + * order to protect mounts against incorrectly setting the
> > + * pipefd we also require that the autofs mount be catatonic.
> > + *
> > + * This also sets the process group id used to identify the
> > + * controlling process (eg. the owning automount(8) daemon).
> > + */
> > +static int autofs_dev_ioctl_setpipefd(struct file *fp,
> > +				      struct autofs_sb_info *sbi,
> > +				      struct autofs_dev_ioctl *param)
> > +{
> > +	int pipefd;
> > +	int err = 0;
> > +
> > +	if (param->arg1 == -1)
> > +		return -EINVAL;
> > +
> > +	pipefd = param->arg1;
> > +
> > +	if (!sbi->catatonic)
> > +		return -EBUSY;
> > +	else {
> > +		struct file *pipe = fget(pipefd);
> > +		if (!pipe->f_op || !pipe->f_op->write) {
> > +			err = -EPIPE;
> > +			fput(pipe);
> > +			goto out;
> > +		}
> > +		sbi->oz_pgrp = task_pgrp_nr(current);
> > +		sbi->pipefd = pipefd;
> > +		sbi->pipe = pipe;
> > +		sbi->catatonic = 0;
> > +	}
> > +out:
> > +	return err;
> > +}
> 
> We have a new filesystem type, a misc device with a mysterious ioctl,
> hand-rolled mountpoint chasing, hand-rolled fd installation and now pipes
> too.
> 
> This is a complex interface.  We really need to see the overall problem
> statement, design and interface description, please.
> 
> > +/*
> > + * Make the autofs mount point catatonic, no longer responsive to
> > + * mount requests. Also closes the kernel pipe file descriptor.
> > + */
> > +static inline int autofs_dev_ioctl_catatonic(struct file *fp,
> > +					     struct autofs_sb_info *sbi,
> > +					     struct autofs_dev_ioctl *param)
> > +{
> > +	if (!sbi->catatonic)
> > +		autofs4_catatonic_mode(sbi);
> > +	return 0;
> > +}
> > +
> > +/* Set the autofs mount timeout */
> > +static inline int autofs_dev_ioctl_timeout(struct file *fp,
> > +					   struct autofs_sb_info *sbi,
> > +					   struct autofs_dev_ioctl *param)
> > +{
> > +	unsigned long timeout;
> > +
> > +	timeout = param->arg1;
> > +	param->arg1 = sbi->exp_timeout / HZ;
> > +	sbi->exp_timeout = timeout * HZ;
> > +	return 0;
> > +}
> 
> uninline everything...
> 
> > +/*
> > + * Return the uid and gid of the last request for the mount
> > + *
> > + * When reconstructing an autofs mount tree with active mounts
> > + * we need to re-connect to mounts that may have used the original
> > + * process uid and gid (or string variations of them) for mount
> > + * lookups within the map entry.
> > + */
> > +static inline int autofs_dev_ioctl_requestor(struct file *fp,
> > +					     struct autofs_sb_info *sbi,
> > +					     struct autofs_dev_ioctl *param)
> 
> especially that - it's only ever called indirectly anwyay!
> 
> > +	struct autofs_info *ino;
> > +	struct nameidata nd;
> > +	const char *path;
> > +	dev_t devid;
> > +	int err = -ENOENT;
> > +
> > +	if (param->size <= sizeof(*param)) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	path = param->path;
> > +	devid = sbi->sb->s_dev;
> > +
> > +	param->arg1 = param->arg2 = -1;
> > +
> > +	/* Get nameidata of the parent directory */
> > +	err = path_lookup(path, LOOKUP_PARENT, &nd);
> > +	if (err)
> > +		goto out;
> > +
> > +	err = autofs_dev_ioctl_find_super(&nd, devid);
> > +	if (err)
> > +		goto out_release;
> > +	
> > +	ino = autofs4_dentry_ino(nd.path.dentry);
> > +	if (ino) {
> > +		err = 0;
> > +		param->arg1 = ino->uid;
> > +		param->arg2 = ino->gid;
> > +	}
> > +
> > +out_release:
> > +	path_put(&nd.path);
> > +out:
> > +	return err;
> > +}
> > +
> > +/*
> > + * Call repeatedly until it returns -EAGAIN, meaning there's nothing
> > + * more that can be done.
> > + */
> > +static int autofs_dev_ioctl_expire(struct file *fp,
> > +				   struct autofs_sb_info *sbi,
> > +				   struct autofs_dev_ioctl *param)
> > +{
> > +	struct dentry *dentry;
> > +	struct vfsmount *mnt;
> > +	int err = -EAGAIN;
> > +	int when;
> > +
> > +	when = param->arg1;
> > +	mnt = fp->f_path.mnt;
> > +
> > +	if (sbi->type & AUTOFS_TYPE_DIRECT)
> > +		dentry = autofs4_expire_direct(sbi->sb, mnt, sbi, when);
> > +	else
> > +		dentry = autofs4_expire_indirect(sbi->sb, mnt, sbi, when);
> > +
> > +	if (dentry) {
> > +		struct autofs_info *ino = autofs4_dentry_ino(dentry);
> > +
> > +		/*
> > +		 * This is synchronous because it makes the daemon a
> > +		 * little easier
> > +		*/
> > +		ino->flags |= AUTOFS_INF_EXPIRING;
> > +		err = autofs4_wait(sbi, dentry, NFY_EXPIRE);
> > +		ino->flags &= ~AUTOFS_INF_EXPIRING;
> 
> Are there races around the modification of ino->flags here?
> 
> > +		dput(dentry);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +/* Check if autofs mount point is in use */
> > +static inline int autofs_dev_ioctl_askumount(struct file *fp,
> > +					     struct autofs_sb_info *sbi,
> > +					     struct autofs_dev_ioctl *param)
> > +{
> > +	param->arg1 = 0;
> > +	if (may_umount(fp->f_path.mnt))
> > +		param->arg1 = 1;
> > +	return 0;
> > +}
> > +
> > +/*
> > + * Check if the given path is a mountpoint.
> > + *
> > + * If we are supplied with the file descriptor of the autofs
> > + * mount we're looking for a specific mount. In this case
> > + * the path is considered a mountpoint if it is itself a
> > + * mountpoint or contains a mount, such as a multi-mount
> > + * without a root mount.
> > + *
> > + * If we aren't supplied with a file descriptor then we lookup
> > + * the nameidata of the path and check if is a mounted autofs
> > + * filesystem or a filesystem mounted within an autofs filesystem.
> > + *
> > + * In both cases we return an indication of whether the path
> > + * is a mount point and the super magic of the covering mount.
> > + */
> > +static inline int autofs_dev_ioctl_ismountpoint(struct file *fp,
> > +						struct autofs_sb_info *sbi,
> > +						struct autofs_dev_ioctl *param)
> 
> uninline..
> 
> > +	struct nameidata nd;
> > +	const char *path;
> > +	int err = -ENOENT;
> > +
> > +	if (param->size <= sizeof(*param)) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> > +
> > +	path = param->path;
> > +
> > +	param->arg1 = param->arg2 = 0;
> > +
> > +	if (param->ioctlfd == -1) {
> > +		unsigned long magic;
> > +
> > +		err = path_lookup(path, LOOKUP_FOLLOW, &nd);
> > +		if (err)
> > +			goto out;
> > +
> > +		magic = nd.path.dentry->d_inode->i_sb->s_magic;
> > +
> > +		if (follow_up(&nd.path.mnt, &nd.path.dentry)) {
> > +			struct inode *inode = nd.path.dentry->d_inode;
> > +			if (magic == AUTOFS_SUPER_MAGIC ||
> > +			    inode->i_sb->s_magic == AUTOFS_SUPER_MAGIC) {
> > +				param->arg1 = 1;
> > +				param->arg2 = magic;
> > +			}
> > +		}
> > +	} else {
> > +		dev_t devid = sbi->sb->s_dev;
> > +
> > +		err = path_lookup(path, LOOKUP_PARENT, &nd);
> > +		if (err)
> > +			goto out;
> > +
> > +		err = autofs_dev_ioctl_find_super(&nd, devid);
> > +		if (err)
> > +			goto out_release;
> > +	
> > +		param->arg1 = have_submounts(nd.path.dentry);
> > +
> > +		if (d_mountpoint(nd.path.dentry)) {
> > +			if (follow_down(&nd.path.mnt, &nd.path.dentry)) {
> > +				struct inode *inode = nd.path.dentry->d_inode;
> > +				param->arg2 = inode->i_sb->s_magic;
> > +			}
> > +		}
> > +	}
> > +
> > +out_release:
> > +	path_put(&nd.path);
> > +out:
> > +	return err;
> > +}
> 
> Have you really carefully reviewed and tested what happens when non-autofs
> fds are fed into all the ioctl modes?
> 
> I hope all these ioctl entrypoints are root-only.  What determines that? 
> The miscdevice permissions?
> 
> > +/*
> > + * Our range of ioctl numbers isn't 0 based so we need to shift
> > + * the array index by _IOC_NR(AUTOFS_CTL_IOC_FIRST) for the table
> > + * lookup.
> > + */
> > +#define cmd_idx(cmd)	(cmd - _IOC_NR(AUTOFS_DEV_IOCTL_IOC_FIRST))
> > +
> > +static ioctl_fn lookup_dev_ioctl(unsigned int cmd)
> > +{
> > +	static struct {
> > +		int cmd;
> > +		ioctl_fn fn;
> > +	} _ioctls[] = {
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_VERSION_CMD), NULL},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_PROTOVER_CMD), autofs_dev_ioctl_protover},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_PROTOSUBVER_CMD), autofs_dev_ioctl_protosubver},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_OPENMOUNT_CMD), autofs_dev_ioctl_openmount},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_CLOSEMOUNT_CMD), autofs_dev_ioctl_closemount},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_READY_CMD), autofs_dev_ioctl_ready},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_FAIL_CMD), autofs_dev_ioctl_fail},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_SETPIPEFD_CMD), autofs_dev_ioctl_setpipefd},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_CATATONIC_CMD), autofs_dev_ioctl_catatonic},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_TIMEOUT_CMD), autofs_dev_ioctl_timeout},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_REQUESTOR_CMD), autofs_dev_ioctl_requestor},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_EXPIRE_CMD), autofs_dev_ioctl_expire},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_ASKUMOUNT_CMD), autofs_dev_ioctl_askumount},
> > +		{cmd_idx(AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD), autofs_dev_ioctl_ismountpoint}
> > +	};
> > +	int idx = cmd_idx(cmd);
> 
> `idx' should have unsigned type.
> 
> > +	return (idx >= ARRAY_SIZE(_ioctls)) ? NULL : _ioctls[idx].fn;
> > +}
> > +
> > +/* ioctl dispatcher */
> > +static int _autofs_dev_ioctl(unsigned int command, struct autofs_dev_ioctl __user *user)
> > +{
> > +	struct autofs_dev_ioctl *param;
> > +	struct file *fp;
> > +	struct autofs_sb_info *sbi;
> > +	unsigned int cmd;
> > +	ioctl_fn fn = NULL;
> > +	int err = 0;
> > +
> > +	/* only root can play with this */
> > +	if (!capable(CAP_SYS_ADMIN))
> > +		return -EPERM;
> 
> OK, I guess that answers my above question.
> 
> > +	cmd = _IOC_NR(command);
> > +
> > +	if (_IOC_TYPE(command) != _IOC_TYPE(AUTOFS_DEV_IOCTL_IOC_FIRST) ||
> > +	    cmd - _IOC_NR(AUTOFS_DEV_IOCTL_IOC_FIRST) >= AUTOFS_DEV_IOCTL_IOC_COUNT) {
> > +		return -ENOTTY;
> > +	}
> > +
> > +	fn = lookup_dev_ioctl(cmd);
> > +	if (!fn) {
> > +		WARN("unknown command 0x%08x", command);
> > +		return -ENOTTY;
> > +	}
> > +
> > +	/*
> > +	 * Trying to avoid low memory issues when a device is
> > +	 * suspended.
> > +	 */
> > +	current->flags |= PF_MEMALLOC;
> 
> whoa, what's this doing here?
> 
> > +	/* Copy the parameters into kernel space. */
> > +	param = copy_dev_ioctl(user);
> > +
> > +	current->flags &= ~PF_MEMALLOC;
> > +
> > +	if (IS_ERR(param))
> > +		return PTR_ERR(param);
> > +
> > +	err = validate_dev_ioctl(command, param);
> > +	if (err)
> > +		goto out;
> > +
> > +	/* The validate routine above always sets the version */
> > +	if (cmd == AUTOFS_DEV_IOCTL_VERSION_CMD)
> > +		goto done;
> > +
> > +	fp = NULL;
> > +	sbi = NULL;
> > +
> > +	/*
> > +	 * For obvious reasons the openmount can't have a file
> > +	 * descriptor yet. We don't take a reference to the
> > +	 * file during close to allow for immediate release.
> > +	 */
> > +	if (cmd != AUTOFS_DEV_IOCTL_OPENMOUNT_CMD &&
> > +	    cmd != AUTOFS_DEV_IOCTL_CLOSEMOUNT_CMD) {
> > +		fp = fget(param->ioctlfd);
> > +		if (!fp) {
> > +			if (cmd == AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD)
> > +				goto cont;
> > +			err = -EBADF;
> > +			goto out;
> > +		}
> > +
> > +		if (!fp->f_op) {
> > +			err = -ENOTTY;
> > +			fput(fp);
> > +			goto out;
> > +		}
> > +
> > +		sbi = autofs_dev_ioctl_sbi(fp);
> > +		if (!sbi) {
> > +			err = -EINVAL;
> > +			fput(fp);
> > +			goto out;
> > +		}
> > +	}
> > +cont:
> > +	err = fn(fp, sbi, param);
> > +
> > +	if (fp)
> > +		fput(fp);
> > +done:
> > +	if (!err && copy_to_user(user, param, AUTOFS_DEV_IOCTL_SIZE))
> > +		err = -EFAULT;
> > +out:
> > +	free_dev_ioctl(param);
> > +	return err;
> > +}
> > +
> >
> > ...
> >
> > diff -up /dev/null linux-2.6.25-rc2-mm1/include/linux/auto_dev-ioctl.h
> > --- /dev/null	2008-02-22 18:55:57.149000956 +0900
> > +++ linux-2.6.25-rc2-mm1/include/linux/auto_dev-ioctl.h	2008-02-22 11:51:41.000000000 +0900
> > @@ -0,0 +1,114 @@
> > +/*
> > + * linux/include/linux/auto_dev-ioctl.h
> > + *
> > + * Copyright 2008 Red Hat, Inc. All rights reserved.
> > + * Copyright 2008 Ian Kent <raven@themaw.net>
> > + *
> > + * This file is part of the Linux kernel and is made available under
> > + * the terms of the GNU General Public License, version 2, or at your
> > + * option, any later version, incorporated herein by reference.
> > + */
> > +
> > +#ifndef _LINUX_AUTO_DEV_IOCTL_H
> > +#define _LINUX_AUTO_DEV_IOCTL_H
> > +
> > +#include <linux/types.h>
> > +
> > +#define AUTOFS_DEV_IOCTL_VERSION_MAJOR	1
> > +#define AUTOFS_DEV_IOCTL_VERSION_MINOR	0
> > +
> > +#define AUTOFS_DEVID_LEN		16
> > +
> > +/*
> > + * An ioctl interface for autofs mount point control.
> > + */
> > +
> > +/*
> > + * All the ioctls use this structure.
> > + * When sending a path size must account for the total length
> > + * of the chunk of memory otherwise is is the size of the
> > + * structure.
> > + */
> > +
> > +struct autofs_dev_ioctl {
> > +	__u32 ver_major;
> > +	__u32 ver_minor;
> > +	__u32 size;		/* total size of data passed in
> > +				 * including this struct */
> > +	__s32 ioctlfd;		/* automount command fd */
> > +
> > +	__u32 arg1;		/* Command parameters */
> > +	__u32 arg2;
> > +
> > +	char path[0];
> > +};
> > +
> > +static inline void init_autofs_dev_ioctl(struct autofs_dev_ioctl *in)
> > +{
> > +	in->ver_major = AUTOFS_DEV_IOCTL_VERSION_MAJOR;
> > +	in->ver_minor = AUTOFS_DEV_IOCTL_VERSION_MINOR;
> > +	in->size = sizeof(struct autofs_dev_ioctl);
> > +	in->ioctlfd = -1;
> > +	in->arg1 = 0;
> > +	in->arg2 = 0;
> > +	return;
> > +}
> 
> uninline..
> 
> > +/*
> > + * If you change this make sure you make the corresponding change
> > + * to autofs-dev-ioctl.c:lookup_ioctl()
> 
> Can we do this automatically via preprocessor tricks, or whatever?
> 
> > + */
> > +enum {
> > +	/* Get various version info */
> > +	AUTOFS_DEV_IOCTL_VERSION_CMD = 0x71,
> > +	AUTOFS_DEV_IOCTL_PROTOVER_CMD,
> > +	AUTOFS_DEV_IOCTL_PROTOSUBVER_CMD,
> > +
> > +	/* Open mount ioctl fd */
> > +	AUTOFS_DEV_IOCTL_OPENMOUNT_CMD,
> > +
> > +	/* Close mount ioctl fd */
> > +	AUTOFS_DEV_IOCTL_CLOSEMOUNT_CMD,
> > +
> > +	/* Mount/expire status returns */
> > +	AUTOFS_DEV_IOCTL_READY_CMD,
> > +	AUTOFS_DEV_IOCTL_FAIL_CMD,
> > +
> > +	/* Activate/deactivate autofs mount */
> > +	AUTOFS_DEV_IOCTL_SETPIPEFD_CMD,
> > +	AUTOFS_DEV_IOCTL_CATATONIC_CMD,
> > +
> > +	/* Expiry timeout */
> > +	AUTOFS_DEV_IOCTL_TIMEOUT_CMD,
> > +
> > +	/* Get mount last requesting uid and gid */
> > +	AUTOFS_DEV_IOCTL_REQUESTOR_CMD,
> > +
> > +	/* Check for eligible expire candidates */
> > +	AUTOFS_DEV_IOCTL_EXPIRE_CMD,
> > +
> > +	/* Request busy status */
> > +	AUTOFS_DEV_IOCTL_ASKUMOUNT_CMD,
> > +
> > +	/* Check if path is a mountpoint */
> > +	AUTOFS_DEV_IOCTL_ISMOUNTPOINT_CMD,
> > +};
> > +
> 


  reply	other threads:[~2008-02-28  6:22 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-26  3:21 [PATCH 0/4] autofs4 - autofs needs a " Ian Kent
2008-02-26  3:22 ` [PATCH 1/4] autofs4 - check for invalid dentry in getpath Ian Kent
2008-02-26  3:23 ` [PATCH 3/4] autofs4 - track uid and gid of last mount requestor Ian Kent
2008-02-26  5:14   ` [PATCH 3/4] autofs4 - track uid and gid of last mount requestor - correction Ian Kent
2008-02-28  4:45   ` [PATCH 3/4] autofs4 - track uid and gid of last mount requestor Andrew Morton
2008-02-28  6:22     ` Ian Kent
2008-02-28  6:37       ` Andrew Morton
2008-02-28  7:08         ` Ian Kent
2008-02-28  7:23           ` Andrew Morton
2008-02-28  8:00             ` Ian Kent
2008-02-28 17:13               ` Jeff Moyer
2008-02-28 19:51                 ` Serge E. Hallyn
2008-02-29  3:32                   ` Ian Kent
2008-02-29 16:09                     ` Serge E. Hallyn
2008-02-29 16:20                       ` Pavel Emelyanov
2008-02-29 17:42                         ` Serge E. Hallyn
2008-03-02  0:49                       ` Eric W. Biederman
2008-03-02  1:13                       ` Eric W. Biederman
2008-03-03 15:28                         ` Serge E. Hallyn
2008-03-04 22:16                           ` Eric W. Biederman
2008-02-28  7:51           ` Pavel Emelyanov
2008-02-28  7:59             ` Andrew Morton
2008-02-28  8:06               ` Ian Kent
2008-02-28 12:31                 ` [autofs] " Fabio Olive Leite
2008-02-28 20:33             ` Eric W. Biederman
2008-02-26  3:23 ` [PATCH 4/4] autofs4 - add miscelaneous device for ioctls Ian Kent
2008-02-28  5:17   ` Andrew Morton
2008-02-28  6:18     ` Ian Kent [this message]
2008-03-13  7:00       ` [RFC] " Ian Kent
2008-03-14  2:45         ` Ian Kent
2008-03-14 12:45         ` Thomas Graf
2008-03-14 14:10           ` Ian Kent
2008-02-29 16:24     ` Ian Kent
2008-04-11  7:02       ` Ian Kent
2008-04-12  4:03         ` Andrew Morton
2008-04-14  4:45           ` Ian Kent
2008-02-26  4:29 ` [PATCH 2/4] autofs4 - add mount option to display mount device Ian Kent
2008-02-28  5:17   ` Andrew Morton
2008-02-28  4:40 ` [PATCH 0/4] autofs4 - autofs needs a miscelaneous device for ioctls Andrew Morton
2008-02-28  6:07   ` Ian Kent
2008-08-07 11:40 [PATCH 1/4] autofs4 - cleanup autofs mount type usage Ian Kent
2008-08-07 11:40 ` [PATCH 4/4] autofs4 - add miscelaneous device for ioctls Ian Kent
2008-08-07 21:10   ` Andrew Morton
2008-08-08  3:39     ` Ian Kent
2008-08-08  5:31       ` Andrew Morton
2008-08-08  6:12         ` Ian Kent
2008-08-08  6:33           ` Andrew Morton
2008-08-09 12:59   ` Christoph Hellwig
2008-08-09 15:29     ` Ian Kent
2008-08-09 17:18       ` Christoph Hellwig
2008-08-10  5:20         ` Ian Kent

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1204179539.3501.17.camel@raven.themaw.net \
    --to=raven@themaw.net \
    --cc=akpm@linux-foundation.org \
    --cc=autofs@linux.kernel.org \
    --cc=hch@lst.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH 4/4] autofs4 - add miscelaneous device for ioctls' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).