| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Sep 2014 12:00:11 +0200
From: Jan Kara <jack@...e.cz>
To: Andrew Vagin <avagin@...allels.com>
Cc: Jan Kara <jack@...e.cz>,
Heinrich Schuchardt <xypron.debian@....de>,
Al Viro <viro@...IV.linux.org.uk>,
Andrey Vagin <avagin@...nvz.org>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
John McCutchan <john@...nmccutchan.com>,
Robert Love <rlove@...ve.org>,
Eric Paris <eparis@...isplace.org>,
Cyrill Gorcunov <gorcunov@...nvz.org>,
Pavel Emelyanov <xemul@...allels.com>,
"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Subject: Re: [PATCH] fs: don't remove inotify watchers from alive inode-s
On Thu 18-09-14 01:01:22, Andrew Vagin wrote:
> From c7a79bccca1aac70f5e50fb145942b932eca79ba Mon Sep 17 00:00:00 2001
> From: Andrey Vagin <avagin@...nvz.org>
> Date: Sat, 6 Sep 2014 08:35:24 +0400
> Subject: [PATCH] fs: don't remove inotify watchers from alive inode-s (v2)
>
> Currently watchers are removed in dentry_iput(), if n_link is zero. But
> other detries can be linked with this inode.
>
> For example if we create two hard links, open the first one and set an
> inotify watcher on one of them. Then if we remove the opened file and
> then another file, the inotify watcher will be removed. But we will have
> the alive file descriptor, which allows us to generate more events.
>
> And here is another behaviour, if files are removed in another order.
> The watcher will not be removed and we will keep getting inotify events
> for that inode.
>
> This patch removes difference of behaviours for these cases. Watchers
> are removed, only if nlink is zero and i_dentry list is empty. The
> resulting behaviour is the same with what has been described in the
> second case.
>
> Look at a following example:
>
> fd = inotify_init1(IN_NONBLOCK);
> deleted = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0666);
> link(path, path_link);
>
> wd_deleted = inotify_add_watch(fd, path_link, IN_ALL_EVENTS);
>
> unlink(path);
> unlink(path_link);
>
> printf(" --- unlink path, path_link\n");
> read_evetns(fd);
>
> close(deleted);
> printf(" --- close\n");
> read_evetns(fd);
> printf(" --- end\n");
>
> We expect to get the same set of events for this case and for the
> case, when files are deleted in another order. But now we get the
> different set of events.
>
> Without this patch:
> The first case, when "path" is deleted before "path_link"
> --- unlink path, path_link
> 4 (IN_ATTRIB)
> 400 (IN_DELETE_SELF)
> 8000 (IN_IGNORED)
> --- close
> --- end
>
> and for the case, when "path_link" is deleted before "path"
> --- unlink path_link, path
> 4 (IN_ATTRIB)
> --- close
> 8 (IN_CLOSE_WRITE)
> 400 (IN_DELETE_SELF)
> 8000 (IN_IGNORED)
> --- end
>
> With this patch we have the same output for both cases:
> --- unlink
> 4 (IN_ATTRIB)
> --- close
> 8 (IN_CLOSE_WRITE)
> 400 (IN_DELETE_SELF)
> 8000 (IN_IGNORED)
> --- end
> PASS
>
> v2: generate IN_DELETE_SELF when the last link to the file is removed
The patch looks good to me and the description in changelog is good so
feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> Cc: Jan Kara <jack@...e.cz>
> Cc: Heinrich Schuchardt <xypron.debian@....de>
> Cc: Alexander Viro <viro@...iv.linux.org.uk>
> Cc: John McCutchan <john@...nmccutchan.com>
> Cc: Robert Love <rlove@...ve.org>
> Cc: Eric Paris <eparis@...isplace.org>
> Cc: Cyrill Gorcunov <gorcunov@...nvz.org>
> Cc: Pavel Emelyanov <xemul@...allels.com>
> Signed-off-by: Andrey Vagin <avagin@...nvz.org>
> ---
> fs/dcache.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/dcache.c b/fs/dcache.c
> index 7a5b514..3a0e3bc 100644
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -278,12 +278,15 @@ static void dentry_iput(struct dentry * dentry)
> __releases(dentry->d_inode->i_lock)
> {
> struct inode *inode = dentry->d_inode;
> + bool last_dentry;
> +
> if (inode) {
> dentry->d_inode = NULL;
> hlist_del_init(&dentry->d_alias);
> + last_dentry = hlist_empty(&inode->i_dentry);
> spin_unlock(&dentry->d_lock);
> spin_unlock(&inode->i_lock);
> - if (!inode->i_nlink)
> + if (!inode->i_nlink && last_dentry)
> fsnotify_inoderemove(inode);
> if (dentry->d_op && dentry->d_op->d_iput)
> dentry->d_op->d_iput(dentry, inode);
> @@ -303,13 +306,16 @@ static void dentry_unlink_inode(struct dentry * dentry)
> __releases(dentry->d_inode->i_lock)
> {
> struct inode *inode = dentry->d_inode;
> + bool last_dentry;
> +
> __d_clear_type(dentry);
> dentry->d_inode = NULL;
> hlist_del_init(&dentry->d_alias);
> dentry_rcuwalk_barrier(dentry);
> + last_dentry = hlist_empty(&inode->i_dentry);
> spin_unlock(&dentry->d_lock);
> spin_unlock(&inode->i_lock);
> - if (!inode->i_nlink)
> + if (!inode->i_nlink && last_dentry)
> fsnotify_inoderemove(inode);
> if (dentry->d_op && dentry->d_op->d_iput)
> dentry->d_op->d_iput(dentry, inode);
> --
> 1.9.3
>
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists