linux-kernel - Re: [PATCH 1/3] fsnotify: tree-watching support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1267472822.10582.104.camel@localhost>
Date:	Mon, 01 Mar 2010 14:47:02 -0500
From:	Eric Paris <eparis@...hat.com>
To:	Joris Dolderer <vorstadtkind@...glemail.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] fsnotify: tree-watching support

On Fri, 2010-02-19 at 14:46 +0100, Joris Dolderer wrote:
> Add tree-watching support to fsnotify.
> See http://patchwork.kernel.org/patch/79171/ and http://patchwork.kernel.org/patch/79172/ for other patches.
> Signed-off-by: Joris Dolderer <vorstadtkind@...glemail.com>
> ---
>  fs/debugfs/inode.c               |    8 +
>  fs/namei.c                       |   12 +-
>  fs/notify/fsnotify.c             |  189 +++++++++++++++++++++++++++++++--------
>  fs/notify/fsnotify.h             |    1 
>  fs/notify/inode_mark.c           |   46 ++++++++-
>  include/linux/dcache.h           |    3 
>  include/linux/fsnotify.h         |   55 +++++++----
>  include/linux/fsnotify_backend.h |   51 +++++++---
>  8 files changed, 282 insertions(+), 83 deletions(-)
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 037e878..17cd902 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -76,55 +76,165 @@ void __fsnotify_update_child_dentry_flags(struct inode *inode)
>  	spin_unlock(&dcache_lock);
>  }
>  
> -/* Notify this dentry's parent about a child's events. */
> -void __fsnotify_parent(struct dentry *dentry, __u32 mask)
> +/*
> + * Does the work when updating descents of a dentry
> + */
> +static void fsnotify_update_descents(struct dentry *dentry, bool watched)
>  {
> -	struct dentry *parent;
> -	struct inode *p_inode;
> -	bool send = false;
> -	bool should_update_children = false;
> +	struct dentry *child;
> +
> +	list_for_each_entry(child, &dentry->d_subdirs, d_u.d_child) {
> +		if (!child->d_inode)
> +			continue;
> +
> +		spin_lock(&child->d_lock);
> +
> +		if (watched)
> +			child->d_flags |= DCACHE_FSNOTIFY_ANCESTOR_WATCHED;
> +		else
> +			child->d_flags &= ~DCACHE_FSNOTIFY_ANCESTOR_WATCHED;
> +
> +		if (!fsnotify_inode_watches_descents(child->d_inode)) {
> +			fsnotify_update_descents(child, watched);
> +		}
> +
> +		spin_unlock(&child->d_lock);
> +	}
> +}

I haven't tested the code at all, just started to even peek at it and
only made it this far.....

Did you run this with lockdep enabled?  Turn it on, understand and fix
anything it complains about.

Any kind of recursion, especially that which is not tail recursion is
highly frowned upon.

What happens when you do

mkdir /tmp/subdir
ln -s ../ /tmp/subdir/upone

and then put a recursive watch on /tmp?

I'm betting deadlock for 1 of 2 reasons, either you

A) try to take the dentry->d_lock on subdir twice (deadlock)
B) run indefinitely around the /tmp -> /tmp/subdir -> /tmp/subdir/upone
-> /tmp/subidr -> /tmp/subdir/upone -> ... loop.

Maybe I'm missing something, but both of these make this a
non-starter....

Maybe symlinks don't break like that, but I'm betting I could do A with
bind mounts if not B.....

-Eric

--
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/