Re: fanotify coredump issue

[Lino Sanfilippo <LinoSanfilippo@gmx.de>]

>
> -------- Original Message --------
> Subject: 	[malware-list] fanotify coredump issue
> Date: 	Mon, 27 Dec 2010 19:52:08 +0300
> From: 	Василий Новиков <vasiliy.novikov@gmail.com>
> To: 	eparis@redhat.com <eparis@redhat.com>
> CC: 	linux-fsdevel@vger.kernel.org <linux-fsdevel@vger.kernel.org>,
> vasily.novikov@kaspersky.com, malware-list@dmesg.printk.net
>
>
>
> Hi Eric,
>
> I tested fanotify in 2.6.37-rc7 and faced the following issue: when a
> fanotify server process (started with open_perm set) segfaults the
> kernel tries to open core dump file and here it is forced to wait a
> permission result because fanotify server receives notifications on
> file operations initiated by itself. Since fanotify server has crashed
> the permission will never be granted. So the whole system hangs.

Hmm I could not reproduce this with the latest state (ef9bf3b7144bee6ce) of  
branch 'origin/for-next' from git.infradead.org/users/eparis/notify.git 

What i did was:
- register with fanotify
- set mark for OPEN_PERM event
- read an event
- cause a segfault before response is returned to fanotify

The process terminates and the core file is created as expected.
Could you provide some code to trigger this?

> I don't see the point in receiving notifications on file operations
> initiated by fanotify server process so I created a patch that
> disables that and solves the issue at least in case of one fanotify
> server.
>
> Best regards,
>  Vasily
>
> /usr/src/linux
> --- ./fs/notify/fanotify/fanotify.c.orig	2010-12-24 12:50:33.653029001 -0500
> +++ ./fs/notify/fanotify/fanotify.c	2010-12-24 13:28:19.561029005 -0500
> @@ -92,6 +92,9 @@ static int fanotify_get_response_from_ac
>  
>  	pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>  
> +	if(group->fanotify_data.tgid == task_tgid(current))
> +		return 0;
> +
>  	wait_event(group->fanotify_data.access_waitq, event->response ||
>  				atomic_read(&group->fanotify_data.bypass_perm));
>  
> @@ -217,6 +220,7 @@ static void fanotify_free_group_priv(str
>  	user = group->fanotify_data.user;
>  	atomic_dec(&user->fanotify_listeners);
>  	free_uid(user);
> +	put_pid(group->fanotify_data.tgid);
>  }
>  
>  const struct fsnotify_ops fanotify_fsnotify_ops = {
> --- ./fs/notify/fanotify/fanotify_user.c.orig	2010-12-24 12:12:07.593029001 -0500
> +++ ./fs/notify/fanotify/fanotify_user.c	2010-12-27 09:26:15.259956001 -0500
> @@ -337,6 +337,10 @@ static ssize_t fanotify_read(struct file
>  			ret = PTR_ERR(kevent);
>  			if (IS_ERR(kevent))
>  				break;
> +			if(kevent->tgid == group->fanotify_data.tgid) {
> +				fsnotify_put_event(kevent);
> +				continue;
> +			}
>  			ret = copy_event_to_user(group, kevent, buf);
>  			fsnotify_put_event(kevent);
>  			if (ret < 0)
> @@ -718,6 +722,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned
>  	INIT_LIST_HEAD(&group->fanotify_data.access_list);
>  	atomic_set(&group->fanotify_data.bypass_perm, 0);
>  #endif
> +	group->fanotify_data.tgid = get_pid(task_tgid(current));
>  	switch (flags & FAN_ALL_CLASS_BITS) {
>  	case FAN_CLASS_NOTIF:
>  		group->priority = FS_PRIO_0;
> --- ./include/linux/fsnotify_backend.h.orig	2010-12-24 12:08:59.965029002 -0500
> +++ ./include/linux/fsnotify_backend.h	2010-12-27 13:05:34.738636001 -0500
> @@ -171,6 +171,7 @@ struct fsnotify_group {
>  			int f_flags;
>  			unsigned int max_marks;
>  			struct user_struct *user;
> +			struct pid *tgid;
>  		} fanotify_data;
>  #endif /* CONFIG_FANOTIFY */
>  	};

I dont think that this will work. 
A fanotify registration is not tracked by its pid, but by its group which
may be valid within several processes (think of fork() after you registered
with fanotify). 
Thus checking for the pid of the process that has registered with fanotify does 
not really make that much sense - the process may terminate right after registration
while the group continues to exist in (an)other process(es). 

Regards,
Lino 

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html