From b32a09db4fb9a87246ba4e7726a979ac4709ad97 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 26 Feb 2008 09:57:11 -0600 Subject: add match_strlcpy() us it to make v9fs make uname and remotename parsing more robust match_strcpy() is a somewhat creepy function: the caller needs to make sure that the destination buffer is big enough, and when he screws up or forgets, match_strcpy() happily overruns the buffer. There's exactly one customer: v9fs_parse_options(). I believe it currently can't overflow its buffer, but that's not exactly obvious. The source string is a substing of the mount options. The kernel silently truncates those to PAGE_SIZE bytes, including the terminating zero. See compat_sys_mount() and do_mount(). The destination buffer is obtained from __getname(), which allocates from name_cachep, which is initialized by vfs_caches_init() for size PATH_MAX. We're safe as long as PATH_MAX <= PAGE_SIZE. PATH_MAX is 4096. As far as I know, the smallest PAGE_SIZE is also 4096. Here's a patch that makes the code a bit more obviously correct. It doesn't depend on PATH_MAX <= PAGE_SIZE. Signed-off-by: Markus Armbruster Cc: Latchesar Ionkov Cc: Jim Meyering Cc: "Randy.Dunlap" Signed-off-by: Andrew Morton Signed-off-by: Eric Van Hensbergen --- fs/9p/v9fs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/9p') diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c index 9b0f0222e8bb..e307fbd34fa0 100644 --- a/fs/9p/v9fs.c +++ b/fs/9p/v9fs.c @@ -125,10 +125,10 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses) v9ses->afid = option; break; case Opt_uname: - match_strcpy(v9ses->uname, &args[0]); + match_strlcpy(v9ses->uname, &args[0], PATH_MAX); break; case Opt_remotename: - match_strcpy(v9ses->aname, &args[0]); + match_strlcpy(v9ses->aname, &args[0], PATH_MAX); break; case Opt_nodevmap: v9ses->nodev = 1; -- cgit v1.2.3 From ee443996a35c1e04f210cafd43d5a98d41e46085 Mon Sep 17 00:00:00 2001 From: Eric Van Hensbergen Date: Wed, 5 Mar 2008 07:08:09 -0600 Subject: 9p: Documentation updates The kernel-doc comments of much of the 9p system have been in disarray since reorganization. This patch fixes those problems, adds additional documentation and a template book which collects the 9p information. Signed-off-by: Eric Van Hensbergen --- fs/9p/fid.h | 15 ++++++++++ fs/9p/v9fs.c | 8 ++++-- fs/9p/v9fs.h | 85 +++++++++++++++++++++++++++++++++++++++---------------- fs/9p/vfs_addr.c | 2 +- fs/9p/vfs_dir.c | 2 +- fs/9p/vfs_file.c | 11 +++---- fs/9p/vfs_inode.c | 50 +++++++++++++++++++++++--------- fs/9p/vfs_super.c | 1 + 8 files changed, 126 insertions(+), 48 deletions(-) (limited to 'fs/9p') diff --git a/fs/9p/fid.h b/fs/9p/fid.h index 26e07df783b9..c3bbd6af996d 100644 --- a/fs/9p/fid.h +++ b/fs/9p/fid.h @@ -22,6 +22,21 @@ #include +/** + * struct v9fs_dentry - 9p private data stored in dentry d_fsdata + * @lock: protects the fidlist + * @fidlist: list of FIDs currently associated with this dentry + * + * This structure defines the 9p private data associated with + * a particular dentry. In particular, this private data is used + * to lookup which 9P FID handle should be used for a particular VFS + * operation. FID handles are associated with dentries instead of + * inodes in order to more closely map functionality to the Plan 9 + * expected behavior for FID reclaimation and tracking. + * + * See Also: Mapping FIDs to Linux VFS model in + * Design and Implementation of the Linux 9P File System documentation + */ struct v9fs_dentry { spinlock_t lock; /* protect fidlist */ struct list_head fidlist; diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c index e307fbd34fa0..79d310c00188 100644 --- a/fs/9p/v9fs.c +++ b/fs/9p/v9fs.c @@ -71,7 +71,6 @@ static match_table_t tokens = { /** * v9fs_parse_options - parse mount options into session structure - * @options: options string passed from mount * @v9ses: existing v9fs session information * */ @@ -256,9 +255,12 @@ void v9fs_session_close(struct v9fs_session_info *v9ses) } /** - * v9fs_session_cancel - mark transport as disconnected - * and cancel all pending requests. + * v9fs_session_cancel - terminate a session + * @v9ses: session to terminate + * + * mark transport as disconnected and cancel all pending requests. */ + void v9fs_session_cancel(struct v9fs_session_info *v9ses) { P9_DPRINTK(P9_DEBUG_ERROR, "cancel session %p\n", v9ses); p9_client_disconnect(v9ses->clnt); diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h index 7d3a1018db52..a7d567192998 100644 --- a/fs/9p/v9fs.h +++ b/fs/9p/v9fs.h @@ -21,18 +21,69 @@ * */ -/* - * Session structure provides information for an opened session - * - */ +/** + * enum p9_session_flags - option flags for each 9P session + * @V9FS_EXTENDED: whether or not to use 9P2000.u extensions + * @V9FS_ACCESS_SINGLE: only the mounting user can access the hierarchy + * @V9FS_ACCESS_USER: a new attach will be issued for every user (default) + * @V9FS_ACCESS_ANY: use a single attach for all users + * @V9FS_ACCESS_MASK: bit mask of different ACCESS options + * + * Session flags reflect options selected by users at mount time + */ +enum p9_session_flags { + V9FS_EXTENDED = 0x01, + V9FS_ACCESS_SINGLE = 0x02, + V9FS_ACCESS_USER = 0x04, + V9FS_ACCESS_ANY = 0x06, + V9FS_ACCESS_MASK = 0x06, +}; + +/* possible values of ->cache */ +/** + * enum p9_cache_modes - user specified cache preferences + * @CACHE_NONE: do not cache data, dentries, or directory contents (default) + * @CACHE_LOOSE: cache data, dentries, and directory contents w/no consistency + * + * eventually support loose, tight, time, session, default always none + */ + +enum p9_cache_modes { + CACHE_NONE, + CACHE_LOOSE, +}; + +/** + * struct v9fs_session_info - per-instance session information + * @flags: session options of type &p9_session_flags + * @nodev: set to 1 to disable device mapping + * @debug: debug level + * @afid: authentication handle + * @cache: cache mode of type &p9_cache_modes + * @options: copy of options string given by user + * @uname: string user name to mount hierarchy as + * @aname: mount specifier for remote hierarchy + * @maxdata: maximum data to be sent/recvd per protocol message + * @dfltuid: default numeric userid to mount hierarchy as + * @dfltgid: default numeric groupid to mount hierarchy as + * @uid: if %V9FS_ACCESS_SINGLE, the numeric uid which mounted the hierarchy + * @clnt: reference to 9P network client instantiated for this session + * @debugfs_dir: reference to debugfs_dir which can be used for add'l debug + * + * This structure holds state for each session instance established during + * a sys_mount() . + * + * Bugs: there seems to be a lot of state which could be condensed and/or + * removed. + */ struct v9fs_session_info { /* options */ - unsigned char flags; /* session flags */ - unsigned char nodev; /* set to 1 if no disable device mapping */ - unsigned short debug; /* debug level */ - unsigned int afid; /* authentication fid */ - unsigned int cache; /* cache mode */ + unsigned char flags; + unsigned char nodev; + unsigned short debug; + unsigned int afid; + unsigned int cache; char *options; /* copy of mount options */ char *uname; /* user name to mount as */ @@ -45,22 +96,6 @@ struct v9fs_session_info { struct dentry *debugfs_dir; }; -/* session flags */ -enum { - V9FS_EXTENDED = 0x01, /* 9P2000.u */ - V9FS_ACCESS_MASK = 0x06, /* access mask */ - V9FS_ACCESS_SINGLE = 0x02, /* only one user can access the files */ - V9FS_ACCESS_USER = 0x04, /* attache per user */ - V9FS_ACCESS_ANY = 0x06, /* use the same attach for all users */ -}; - -/* possible values of ->cache */ -/* eventually support loose, tight, time, session, default always none */ -enum { - CACHE_NONE, /* default */ - CACHE_LOOSE, /* no consistency */ -}; - extern struct dentry *v9fs_debugfs_root; struct p9_fid *v9fs_session_init(struct v9fs_session_info *, const char *, diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c index 6248f0e727a3..97d3aed57983 100644 --- a/fs/9p/vfs_addr.c +++ b/fs/9p/vfs_addr.c @@ -43,7 +43,7 @@ /** * v9fs_vfs_readpage - read an entire page in from 9P * - * @file: file being read + * @filp: file being read * @page: structure to page * */ diff --git a/fs/9p/vfs_dir.c b/fs/9p/vfs_dir.c index 0924d4477da3..88e3787c6ea9 100644 --- a/fs/9p/vfs_dir.c +++ b/fs/9p/vfs_dir.c @@ -60,7 +60,7 @@ static inline int dt_type(struct p9_stat *mistat) /** * v9fs_dir_readdir - read a directory - * @filep: opened file structure + * @filp: opened file structure * @dirent: directory structure ??? * @filldir: function to populate directory structure ??? * diff --git a/fs/9p/vfs_file.c b/fs/9p/vfs_file.c index a616fff8906d..0d55affe37d4 100644 --- a/fs/9p/vfs_file.c +++ b/fs/9p/vfs_file.c @@ -90,10 +90,11 @@ int v9fs_file_open(struct inode *inode, struct file *file) /** * v9fs_file_lock - lock a file (or directory) - * @inode: inode to be opened - * @file: file being opened + * @filp: file to be locked + * @cmd: lock command + * @fl: file lock structure * - * XXX - this looks like a local only lock, we should extend into 9P + * Bugs: this looks like a local only lock, we should extend into 9P * by using open exclusive */ @@ -118,7 +119,7 @@ static int v9fs_file_lock(struct file *filp, int cmd, struct file_lock *fl) /** * v9fs_file_read - read from a file - * @filep: file pointer to read + * @filp: file pointer to read * @data: data buffer to read data into * @count: size of buffer * @offset: offset at which to read data @@ -142,7 +143,7 @@ v9fs_file_read(struct file *filp, char __user * data, size_t count, /** * v9fs_file_write - write to a file - * @filep: file pointer to write + * @filp: file pointer to write * @data: data buffer to write data from * @count: size of buffer * @offset: offset at which to write data diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 6a28842052ea..40fa807bd929 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -129,6 +129,12 @@ static int p9mode2unixmode(struct v9fs_session_info *v9ses, int mode) return res; } +/** + * v9fs_uflags2omode- convert posix open flags to plan 9 mode bits + * @uflags: flags to convert + * + */ + int v9fs_uflags2omode(int uflags) { int ret; @@ -312,6 +318,14 @@ error: } */ +/** + * v9fs_inode_from_fid - populate an inode by issuing a attribute request + * @v9ses: session information + * @fid: fid to issue attribute request for + * @sb: superblock on which to create inode + * + */ + static struct inode * v9fs_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid, struct super_block *sb) @@ -384,9 +398,12 @@ v9fs_open_created(struct inode *inode, struct file *file) /** * v9fs_create - Create a file + * @v9ses: session information + * @dir: directory that dentry is being created in * @dentry: dentry that is being created * @perm: create permissions * @mode: open mode + * @extension: 9p2000.u extension string to support devices, etc. * */ static struct p9_fid * @@ -461,7 +478,7 @@ error: /** * v9fs_vfs_create - VFS hook to create files - * @inode: directory inode that is being created + * @dir: directory inode that is being created * @dentry: dentry that is being deleted * @mode: create permissions * @nd: path information @@ -519,7 +536,7 @@ error: /** * v9fs_vfs_mkdir - VFS mkdir hook to create a directory - * @inode: inode that is being unlinked + * @dir: inode that is being unlinked * @dentry: dentry that is being unlinked * @mode: mode for new directory * @@ -703,9 +720,9 @@ done: /** * v9fs_vfs_getattr - retrieve file metadata - * @mnt - mount information - * @dentry - file to get attributes on - * @stat - metadata structure to populate + * @mnt: mount information + * @dentry: file to get attributes on + * @stat: metadata structure to populate * */ @@ -928,7 +945,7 @@ done: /** * v9fs_vfs_readlink - read a symlink's location * @dentry: dentry for symlink - * @buf: buffer to load symlink location into + * @buffer: buffer to load symlink location into * @buflen: length of buffer * */ @@ -996,10 +1013,12 @@ static void *v9fs_vfs_follow_link(struct dentry *dentry, struct nameidata *nd) * v9fs_vfs_put_link - release a symlink path * @dentry: dentry for symlink * @nd: nameidata + * @p: unused * */ -static void v9fs_vfs_put_link(struct dentry *dentry, struct nameidata *nd, void *p) +static void +v9fs_vfs_put_link(struct dentry *dentry, struct nameidata *nd, void *p) { char *s = nd_get_link(nd); @@ -1008,6 +1027,15 @@ static void v9fs_vfs_put_link(struct dentry *dentry, struct nameidata *nd, void __putname(s); } +/** + * v9fs_vfs_mkspecial - create a special file + * @dir: inode to create special file in + * @dentry: dentry to create + * @mode: mode to create special file + * @extension: 9p2000.u format extension string representing special file + * + */ + static int v9fs_vfs_mkspecial(struct inode *dir, struct dentry *dentry, int mode, const char *extension) { @@ -1037,7 +1065,7 @@ static int v9fs_vfs_mkspecial(struct inode *dir, struct dentry *dentry, * @dentry: dentry for symlink * @symname: symlink data * - * See 9P2000.u RFC for more information + * See Also: 9P2000.u RFC for more information * */ @@ -1058,10 +1086,6 @@ v9fs_vfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname) * */ -/* XXX - lots of code dup'd from symlink and creates, - * figure out a better reuse strategy - */ - static int v9fs_vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) @@ -1098,7 +1122,7 @@ clunk_fid: * @dir: inode destination for new link * @dentry: dentry for file * @mode: mode for creation - * @dev_t: device associated with special file + * @rdev: device associated with special file * */ diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c index a452ac67fc94..ba10f172626d 100644 --- a/fs/9p/vfs_super.c +++ b/fs/9p/vfs_super.c @@ -75,6 +75,7 @@ static int v9fs_set_super(struct super_block *s, void *data) * v9fs_fill_super - populate superblock with info * @sb: superblock * @v9ses: session information + * @flags: flags propagated from v9fs_get_sb() * */ -- cgit v1.2.3 From ab31267dfeddf80b2e483f077c8b03905993722b Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Thu, 6 Mar 2008 17:10:28 -0600 Subject: fs/9p/v9fs.c (v9fs_parse_options): Handle kstrdup and match_strdup failure. Now that this function can fail, return an int, diagnose other option-parsing failures, and adjust the sole caller: (v9fs_session_init): Handle kstrdup failure. Propagate any new v9fs_parse_options failure "up". Signed-off-by: Jim Meyering Cc: Ron Minnich Cc: Latchesar Ionkov Signed-off-by: Andrew Morton Acked-by: Eric Van Hensbergen --- fs/9p/v9fs.c | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) (limited to 'fs/9p') diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c index 79d310c00188..5c1ccaf0416c 100644 --- a/fs/9p/v9fs.c +++ b/fs/9p/v9fs.c @@ -73,16 +73,17 @@ static match_table_t tokens = { * v9fs_parse_options - parse mount options into session structure * @v9ses: existing v9fs session information * + * Return 0 upon success, -ERRNO upon failure. */ -static void v9fs_parse_options(struct v9fs_session_info *v9ses) +static int v9fs_parse_options(struct v9fs_session_info *v9ses) { char *options; substring_t args[MAX_OPT_ARGS]; char *p; int option = 0; char *s, *e; - int ret; + int ret = 0; /* setup defaults */ v9ses->afid = ~0; @@ -90,19 +91,26 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses) v9ses->cache = 0; if (!v9ses->options) - return; + return 0; options = kstrdup(v9ses->options, GFP_KERNEL); + if (!options) { + P9_DPRINTK(P9_DEBUG_ERROR, + "failed to allocate copy of option string\n"); + return -ENOMEM; + } + while ((p = strsep(&options, ",")) != NULL) { int token; if (!*p) continue; token = match_token(p, tokens, args); if (token < Opt_uname) { - ret = match_int(&args[0], &option); - if (ret < 0) { + int r = match_int(&args[0], &option); + if (r < 0) { P9_DPRINTK(P9_DEBUG_ERROR, "integer field, but no integer?\n"); + ret = r; continue; } } @@ -138,6 +146,13 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses) case Opt_access: s = match_strdup(&args[0]); + if (!s) { + P9_DPRINTK(P9_DEBUG_ERROR, + "failed to allocate copy" + " of option argument\n"); + ret = -ENOMEM; + break; + } v9ses->flags &= ~V9FS_ACCESS_MASK; if (strcmp(s, "user") == 0) v9ses->flags |= V9FS_ACCESS_USER; @@ -157,6 +172,7 @@ static void v9fs_parse_options(struct v9fs_session_info *v9ses) } } kfree(options); + return ret; } /** @@ -172,6 +188,7 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses, { int retval = -EINVAL; struct p9_fid *fid; + int rc; v9ses->uname = __getname(); if (!v9ses->uname) @@ -190,7 +207,18 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses, v9ses->dfltuid = V9FS_DEFUID; v9ses->dfltgid = V9FS_DEFGID; v9ses->options = kstrdup(data, GFP_KERNEL); - v9fs_parse_options(v9ses); + if (!v9ses->options) { + P9_DPRINTK(P9_DEBUG_ERROR, + "failed to allocate copy of option string\n"); + retval = -ENOMEM; + goto error; + } + + rc = v9fs_parse_options(v9ses); + if (rc < 0) { + retval = rc; + goto error; + } v9ses->clnt = p9_client_create(dev_name, v9ses->options); -- cgit v1.2.3 From 887b3ece65be7b643dfdae0d433c91a26a3f437d Mon Sep 17 00:00:00 2001 From: Eric Van Hensbergen Date: Thu, 8 May 2008 20:26:37 -0500 Subject: 9p: fix error path during early mount There was some cleanup issues during early mount which would trigger a kernel bug for certain types of failure. This patch reorganizes the cleanup to get rid of the bad behavior. This also merges the 9pnet and 9pnet_fd modules for the purpose of configuration and initialization. Keeping the fd transport separate from the core 9pnet code seemed like a good idea at the time, but in practice has caused more harm and confusion than good. Signed-off-by: Eric Van Hensbergen --- fs/9p/v9fs.c | 13 +++++++------ fs/9p/vfs_super.c | 34 ++++++++++++++++------------------ 2 files changed, 23 insertions(+), 24 deletions(-) (limited to 'fs/9p') diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c index 5c1ccaf0416c..047c791427aa 100644 --- a/fs/9p/v9fs.c +++ b/fs/9p/v9fs.c @@ -206,12 +206,14 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses, v9ses->uid = ~0; v9ses->dfltuid = V9FS_DEFUID; v9ses->dfltgid = V9FS_DEFGID; - v9ses->options = kstrdup(data, GFP_KERNEL); - if (!v9ses->options) { - P9_DPRINTK(P9_DEBUG_ERROR, + if (data) { + v9ses->options = kstrdup(data, GFP_KERNEL); + if (!v9ses->options) { + P9_DPRINTK(P9_DEBUG_ERROR, "failed to allocate copy of option string\n"); - retval = -ENOMEM; - goto error; + retval = -ENOMEM; + goto error; + } } rc = v9fs_parse_options(v9ses); @@ -260,7 +262,6 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses, return fid; error: - v9fs_session_close(v9ses); return ERR_PTR(retval); } diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c index ba10f172626d..bf59c3960494 100644 --- a/fs/9p/vfs_super.c +++ b/fs/9p/vfs_super.c @@ -128,29 +128,26 @@ static int v9fs_get_sb(struct file_system_type *fs_type, int flags, fid = v9fs_session_init(v9ses, dev_name, data); if (IS_ERR(fid)) { retval = PTR_ERR(fid); - fid = NULL; - kfree(v9ses); - v9ses = NULL; - goto error; + goto close_session; } st = p9_client_stat(fid); if (IS_ERR(st)) { retval = PTR_ERR(st); - goto error; + goto clunk_fid; } sb = sget(fs_type, NULL, v9fs_set_super, v9ses); if (IS_ERR(sb)) { retval = PTR_ERR(sb); - goto error; + goto free_stat; } v9fs_fill_super(sb, v9ses, flags); inode = v9fs_get_inode(sb, S_IFDIR | mode); if (IS_ERR(inode)) { retval = PTR_ERR(inode); - goto error; + goto release_sb; } inode->i_uid = uid; @@ -159,7 +156,7 @@ static int v9fs_get_sb(struct file_system_type *fs_type, int flags, root = d_alloc_root(inode); if (!root) { retval = -ENOMEM; - goto error; + goto release_sb; } sb->s_root = root; @@ -170,21 +167,22 @@ static int v9fs_get_sb(struct file_system_type *fs_type, int flags, return simple_set_mnt(mnt, sb); -error: - kfree(st); - if (fid) - p9_client_clunk(fid); - - if (v9ses) { - v9fs_session_close(v9ses); - kfree(v9ses); - } - +release_sb: if (sb) { up_write(&sb->s_umount); deactivate_super(sb); } +free_stat: + kfree(st); + +clunk_fid: + p9_client_clunk(fid); + +close_session: + v9fs_session_close(v9ses); + kfree(v9ses); + return retval; } -- cgit v1.2.3