From: Al Viro <viro@ZenIV.linux.org.uk> To: Eric Van Hensbergen <ericvh@gmail.com> Cc: Dominique Martinet <dominique.martinet@cea.fr>, Linux FS Devel <linux-fsdevel@vger.kernel.org>, Bug 1336794 <1336794@bugs.launchpad.net>, qemu-devel <qemu-devel@nongnu.org>, V9FS Developers <v9fs-developer@lists.sourceforge.net> Subject: Re: [Qemu-devel] [V9fs-developer] [Bug 1336794] Re: 9pfs does not honor open file handles on unlinked files Date: Tue, 14 Apr 2015 17:07:38 +0100 [thread overview] Message-ID: <20150414160737.GX889@ZenIV.linux.org.uk> (raw) In-Reply-To: <CAFkjPTmwXgq6jJow=zBEuCzJ46mApOF5w8ETbZzh7Eycfg4hhQ@mail.gmail.com> On Mon, Apr 13, 2015 at 04:05:28PM +0000, Eric Van Hensbergen wrote: > Well, technically fid 3 isn't 'open', only fid 2 is open - at least > according to the protocol. fid 3 and fid 2 are both clones of fid 1. > However, thanks for the alternative workaround. If you get a chance, can > you check that my change to the client to partially fix this for the other > servers doesn't break nfs-ganesha: > > https://github.com/ericvh/linux/commit/fddc7721d6d19e4e6be4905f37ade5b0521f4ed5 BTW, what the hell is going on in v9fs_vfs_mknod() and v9fs_vfs_link()? You allocate 4Kb buffer, sprintf into it ("b %u %u", "c %u %u", or "%d\n") feed it to v9fs_vfs_mkspecial() and immediately free it. What's wrong with a local array of char? In the worst case it needs to be char name[24] - surely, we are not so tight on stack that extra 16 bytes (that array instead of a pointer) would drive us over the cliff? IOW, do you have any problem with this: diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 703342e..cda68f7 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -1370,6 +1370,8 @@ v9fs_vfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname) return v9fs_vfs_mkspecial(dir, dentry, P9_DMSYMLINK, symname); } +#define U32_MAX_DIGITS 10 + /** * v9fs_vfs_link - create a hardlink * @old_dentry: dentry for file to link to @@ -1383,7 +1385,7 @@ v9fs_vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) { int retval; - char *name; + char name[1 + U32_MAX_DIGITS + 2]; /* sign + number + \n + \0 */ struct p9_fid *oldfid; p9_debug(P9_DEBUG_VFS, " %lu,%pd,%pd\n", @@ -1393,20 +1395,12 @@ v9fs_vfs_link(struct dentry *old_dentry, struct inode *dir, if (IS_ERR(oldfid)) return PTR_ERR(oldfid); - name = __getname(); - if (unlikely(!name)) { - retval = -ENOMEM; - goto clunk_fid; - } - sprintf(name, "%d\n", oldfid->fid); retval = v9fs_vfs_mkspecial(dir, dentry, P9_DMLINK, name); - __putname(name); if (!retval) { v9fs_refresh_inode(oldfid, d_inode(old_dentry)); v9fs_invalidate_inode_attr(dir); } -clunk_fid: p9_client_clunk(oldfid); return retval; } @@ -1425,7 +1419,7 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rde { struct v9fs_session_info *v9ses = v9fs_inode2v9ses(dir); int retval; - char *name; + char name[2 + U32_MAX_DIGITS + 1 + U32_MAX_DIGITS + 1]; u32 perm; p9_debug(P9_DEBUG_VFS, " %lu,%pd mode: %hx MAJOR: %u MINOR: %u\n", @@ -1435,26 +1429,16 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rde if (!new_valid_dev(rdev)) return -EINVAL; - name = __getname(); - if (!name) - return -ENOMEM; /* build extension */ if (S_ISBLK(mode)) sprintf(name, "b %u %u", MAJOR(rdev), MINOR(rdev)); else if (S_ISCHR(mode)) sprintf(name, "c %u %u", MAJOR(rdev), MINOR(rdev)); - else if (S_ISFIFO(mode)) - *name = 0; - else if (S_ISSOCK(mode)) + else *name = 0; - else { - __putname(name); - return -EINVAL; - } perm = unixmode2p9mode(v9ses, mode); retval = v9fs_vfs_mkspecial(dir, dentry, perm, name); - __putname(name); return retval; }
WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@ZenIV.linux.org.uk> To: Eric Van Hensbergen <ericvh@gmail.com> Cc: Linux FS Devel <linux-fsdevel@vger.kernel.org>, Dominique Martinet <dominique.martinet@cea.fr>, V9FS Developers <v9fs-developer@lists.sourceforge.net>, qemu-devel <qemu-devel@nongnu.org>, Bug 1336794 <1336794@bugs.launchpad.net> Subject: Re: [Qemu-devel] [V9fs-developer] [Bug 1336794] Re: 9pfs does not honor open file handles on unlinked files Date: Tue, 14 Apr 2015 17:07:38 +0100 [thread overview] Message-ID: <20150414160737.GX889@ZenIV.linux.org.uk> (raw) In-Reply-To: <CAFkjPTmwXgq6jJow=zBEuCzJ46mApOF5w8ETbZzh7Eycfg4hhQ@mail.gmail.com> On Mon, Apr 13, 2015 at 04:05:28PM +0000, Eric Van Hensbergen wrote: > Well, technically fid 3 isn't 'open', only fid 2 is open - at least > according to the protocol. fid 3 and fid 2 are both clones of fid 1. > However, thanks for the alternative workaround. If you get a chance, can > you check that my change to the client to partially fix this for the other > servers doesn't break nfs-ganesha: > > https://github.com/ericvh/linux/commit/fddc7721d6d19e4e6be4905f37ade5b0521f4ed5 BTW, what the hell is going on in v9fs_vfs_mknod() and v9fs_vfs_link()? You allocate 4Kb buffer, sprintf into it ("b %u %u", "c %u %u", or "%d\n") feed it to v9fs_vfs_mkspecial() and immediately free it. What's wrong with a local array of char? In the worst case it needs to be char name[24] - surely, we are not so tight on stack that extra 16 bytes (that array instead of a pointer) would drive us over the cliff? IOW, do you have any problem with this: diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c index 703342e..cda68f7 100644 --- a/fs/9p/vfs_inode.c +++ b/fs/9p/vfs_inode.c @@ -1370,6 +1370,8 @@ v9fs_vfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname) return v9fs_vfs_mkspecial(dir, dentry, P9_DMSYMLINK, symname); } +#define U32_MAX_DIGITS 10 + /** * v9fs_vfs_link - create a hardlink * @old_dentry: dentry for file to link to @@ -1383,7 +1385,7 @@ v9fs_vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) { int retval; - char *name; + char name[1 + U32_MAX_DIGITS + 2]; /* sign + number + \n + \0 */ struct p9_fid *oldfid; p9_debug(P9_DEBUG_VFS, " %lu,%pd,%pd\n", @@ -1393,20 +1395,12 @@ v9fs_vfs_link(struct dentry *old_dentry, struct inode *dir, if (IS_ERR(oldfid)) return PTR_ERR(oldfid); - name = __getname(); - if (unlikely(!name)) { - retval = -ENOMEM; - goto clunk_fid; - } - sprintf(name, "%d\n", oldfid->fid); retval = v9fs_vfs_mkspecial(dir, dentry, P9_DMLINK, name); - __putname(name); if (!retval) { v9fs_refresh_inode(oldfid, d_inode(old_dentry)); v9fs_invalidate_inode_attr(dir); } -clunk_fid: p9_client_clunk(oldfid); return retval; } @@ -1425,7 +1419,7 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rde { struct v9fs_session_info *v9ses = v9fs_inode2v9ses(dir); int retval; - char *name; + char name[2 + U32_MAX_DIGITS + 1 + U32_MAX_DIGITS + 1]; u32 perm; p9_debug(P9_DEBUG_VFS, " %lu,%pd mode: %hx MAJOR: %u MINOR: %u\n", @@ -1435,26 +1429,16 @@ v9fs_vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t rde if (!new_valid_dev(rdev)) return -EINVAL; - name = __getname(); - if (!name) - return -ENOMEM; /* build extension */ if (S_ISBLK(mode)) sprintf(name, "b %u %u", MAJOR(rdev), MINOR(rdev)); else if (S_ISCHR(mode)) sprintf(name, "c %u %u", MAJOR(rdev), MINOR(rdev)); - else if (S_ISFIFO(mode)) - *name = 0; - else if (S_ISSOCK(mode)) + else *name = 0; - else { - __putname(name); - return -EINVAL; - } perm = unixmode2p9mode(v9ses, mode); retval = v9fs_vfs_mkspecial(dir, dentry, perm, name); - __putname(name); return retval; }
next prev parent reply other threads:[~2015-04-14 16:07 UTC|newest] Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-07-02 13:52 [Qemu-devel] [Bug 1336794] [NEW] 9pfs does not honor open file handles on unlinked files Cole Robinson 2015-04-10 12:30 ` [Qemu-devel] [Bug 1336794] " Mark Glines 2015-04-12 12:42 ` Eric Van Hensbergen 2015-04-12 12:42 ` [Qemu-devel] " Eric Van Hensbergen 2015-04-12 14:09 ` Al Viro 2015-04-12 19:08 ` Eric Van Hensbergen 2015-04-13 8:27 ` [V9fs-developer] " Dominique Martinet 2015-04-13 8:27 ` [Qemu-devel] " Dominique Martinet 2015-04-13 16:05 ` Eric Van Hensbergen 2015-04-13 16:05 ` [Qemu-devel] " Eric Van Hensbergen 2015-04-14 16:07 ` Al Viro [this message] 2015-04-14 16:07 ` Al Viro 2015-04-14 16:19 ` Eric Van Hensbergen 2015-04-14 16:19 ` [Qemu-devel] " Eric Van Hensbergen 2015-04-14 21:44 ` Al Viro 2015-04-14 21:44 ` Al Viro 2015-04-15 11:28 ` Dominique Martinet 2015-04-15 11:28 ` [Qemu-devel] " Dominique Martinet 2015-04-15 14:17 ` Eric Van Hensbergen 2015-04-15 14:17 ` [Qemu-devel] " Eric Van Hensbergen 2015-04-12 12:45 ` [Qemu-devel] " Eric Van Hensbergen 2016-05-05 20:54 ` Server Angels 2016-05-25 11:14 ` Greg Kurz 2016-05-25 11:51 ` Sean Keeney 2016-05-05 21:01 ` Server Angels 2016-05-24 14:43 ` Launchpad Bug Tracker 2016-06-02 11:59 ` Greg Kurz 2016-06-25 8:25 ` Greg Kurz 2016-06-25 9:24 ` Greg Kurz 2017-08-04 6:03 ` Maxim Kuvyrkov 2017-08-04 8:00 ` Greg Kurz 2018-11-26 21:47 ` Alexander Gretha 2018-11-27 8:07 ` Greg Kurz 2018-11-28 21:20 ` Alexander Gretha 2020-05-26 11:58 ` Greg Kurz 2020-05-27 6:03 ` Greg Kurz 2020-12-10 8:47 ` Thomas Huth 2020-12-10 11:48 ` Thomas Huth 2021-05-03 16:40 ` Thomas Huth
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=20150414160737.GX889@ZenIV.linux.org.uk \ --to=viro@zeniv.linux.org.uk \ --cc=1336794@bugs.launchpad.net \ --cc=dominique.martinet@cea.fr \ --cc=ericvh@gmail.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=qemu-devel@nongnu.org \ --cc=v9fs-developer@lists.sourceforge.net \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.