fanotify coredump issue

fanotify coredump issue

[Василий Новиков]

[-- Attachment #1: Type: text/plain, Size: 651 bytes --]

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.

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

[-- Attachment #2: fanotify_core.patch --]
[-- Type: text/x-patch, Size: 1902 bytes --]

/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 */
 	};

Re: fanotify coredump issue

[Lino Sanfilippo]

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

Re: fanotify coredump issue

[Vasily Novikov]

[-- Attachment #1: Type: text/plain, Size: 1481 bytes --]

Lino,

On 01/04/2011 01:28 PM, Lino Sanfilippo wrote:
>> 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?
>

The issue is still reproduced on ubuntu 11.04:

$ uname -a
Linux user-virtual-machine 2.6.38.2 #16 SMP Wed Jun 8 14:40:03 MSD 2011 
i686 i686 i386 GNU/Linux
$ g++ fanotify.c -o fanotify -lpthread
$ sudo su
$ ulimit -c unlimited
$ ./fanotify -o open_perm -C -m /

In other console:
$ ls

Then machine is dead.

-- 
Best regards,

Vasily Novikov | Software developer | Kaspersky Lab

Direct: +7 495 123 45 67 x2344 | Mobile: +7 964 786 44 82 | 
vasily.novikov@kaspersky.com
10/1, 1st Volokolamsky Proezd, Moscow, 123060, Russia | 
www.kaspersky.com,  www.securelist.com

[-- Attachment #2: fanotify.c --]
[-- Type: text/x-csrc, Size: 7217 bytes --]

#define _ATFILE_SOURCE
#include <errno.h>
#include <inttypes.h>
#include <fcntl.h>
#include <linux/limits.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/select.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <unistd.h>
#include <pthread.h>

#include <linux/fanotify.h>
#include <sys/fanotify.h>
//#include <linux/fsnotify_backend.h>

//#include "fanotify-syscalllib.h"

#define FANOTIFY_ARGUMENTS "cfhmn"

uint64_t fan_mask = FAN_OPEN | FAN_CLOSE | FAN_ACCESS | FAN_MODIFY;
bool opt_fast;
int fan_fd;

int mark_object(int fan_fd, const char *path, int fd, uint64_t mask, unsigned int flags)
{
	return fanotify_mark(fan_fd, flags, mask, fd, path);
}

int set_ignored_mask(int fan_fd, int fd, uint64_t mask)
{
	unsigned int flags = (FAN_MARK_ADD | FAN_MARK_IGNORED_MASK);

	return mark_object(fan_fd, NULL, fd, mask, flags);
}

int handle_perm(int fan_fd, struct fanotify_event_metadata *metadata, unsigned int response)
{
	struct fanotify_response response_struct;
	int ret;

	response_struct.fd = metadata->fd;
	response_struct.response = response;

	ret = write(fan_fd, &response_struct, sizeof(response_struct));
	if (ret < 0)
		return ret;

	return 0;
}

void synopsis(const char *progname, int status)
{
	FILE *file = status ? stderr : stdout;

	fprintf(file, "USAGE: %s [-" FANOTIFY_ARGUMENTS "] "
		"[-o {open,close,access,modify,open_perm,access_perm}] "
		"file ...\n"
		"-c: learn about events on children of a directory (not decendants)\n"
		"-f: set premptive ignores (go faster)\n"
		"-h: this help screen\n"
		"-m: place mark on the whole mount point, not just the inode\n"
		"-n: do not ignore repeated permission checks\n"
		"-s N: sleep N seconds before replying to perm events\n",
		"-C: crash\n",
		progname);
	exit(status);
}

#define FAIL() do { fprintf(stderr, "%s:%d: ", __FILE__, __LINE__); goto fail; } while(0);

int g_N = 30;

void* process_request(void *p)
{
	struct fanotify_event_metadata *metadata = (struct fanotify_event_metadata*)p;
	bool opt_ignore_perm = 1;
	unsigned int response = FAN_ALLOW;
	int opt_sleep = 0;

	pthread_detach(pthread_self());
	
	if (metadata->fd >= 0 &&
			opt_fast &&
			set_ignored_mask(fan_fd, metadata->fd,
				FAN_ALL_EVENTS | FAN_ALL_PERM_EVENTS))
		FAIL();

	if (metadata->fd >= 0) {
		char path[PATH_MAX];
		int path_len;

		sprintf(path, "/proc/self/fd/%d", metadata->fd);
		path_len = readlink(path, path, sizeof(path)-1);
		opt_ignore_perm =  (path[path_len-1] != 'i');
		if(!opt_ignore_perm) printf(" nocache");
		if(path[path_len-1] == 'f') {
			printf(" flush");
//			open("/tmp/123.txt", O_RDONLY);
			if(mark_object(fan_fd, "/tmp", AT_FDCWD, fan_mask, FAN_MARK_FLUSH) != 0) FAIL();
			//	if(mark_object(fan_fd, "/tmp", AT_FDCWD, fan_mask, FAN_MARK_ADD) != 0) FAIL();
		}
		if(path[path_len-1] == 'd') {
			printf(" deny");
			response = FAN_DENY;
		}
		if(path[path_len-1] == 's') {
			printf(" sleep");
			opt_sleep = 1;
		}
		if (path_len < 0)
			FAIL();
		path[path_len] = '\0';
		printf("%s:", path);
	} else
		printf("?:");

	printf(" pid=%ld", (long)metadata->pid);

	if (metadata->mask & FAN_ACCESS)
		printf(" access");
	if (metadata->mask & FAN_OPEN)
		printf(" open");
	if (metadata->mask & FAN_MODIFY)
		printf(" modify");
	if (metadata->mask & FAN_CLOSE) {
		if (metadata->mask & FAN_CLOSE_WRITE)
			printf(" close(writable)");
		else
			printf(" close");
	}
	if (metadata->mask & FAN_OPEN_PERM)
		printf(" open_perm");
	if (metadata->mask & FAN_ACCESS_PERM)
		printf(" access_perm");
	printf(" md->mask:%x PE:%x", (int)metadata->mask, (int)(FAN_ALL_PERM_EVENTS));
	if (metadata->mask & (FAN_ALL_PERM_EVENTS)) {
		if (opt_sleep) {
			sleep(opt_sleep);
		}

		if (metadata->fd >= 0 &&
				opt_ignore_perm &&
				set_ignored_mask(fan_fd, metadata->fd,
					metadata->mask))
			FAIL();
		if (handle_perm(fan_fd, metadata, response))
			FAIL();
	}

	printf("\n");
	fflush(stdout);

	if (metadata->fd >= 0 && close(metadata->fd) != 0)
		FAIL();

	free(p);
	return NULL;

fail:
	fprintf(stderr, "%s\n", strerror(errno));
	exit(1);
	return NULL;
}


void deffered_processing(struct fanotify_event_metadata *a_md)
{
	struct fanotify_event_metadata *md = (struct fanotify_event_metadata *)malloc(sizeof(struct fanotify_event_metadata));
	pthread_t tid;

	if(NULL == md) {
		printf("can't allocate memory for mrequest\n");
		process_request(md);
		return;
	}

	*md = *a_md;

	if(0 != pthread_create(&tid, NULL, process_request, md)) {
		printf("can't create thread\n");
		process_request(md);
	}
}

int main(int argc, char *argv[])
{
	int opt;
	unsigned int mark_flags = FAN_MARK_ADD;
	bool opt_child, opt_on_mount, opt_ignore_perm, crash;
	ssize_t len;
	char buf[4096];
	fd_set rfds;


	opt_child = opt_on_mount = opt_fast = crash = false;
	opt_ignore_perm = true;
//	opt_sleep = 0;

	while ((opt = getopt(argc, argv, "o:s:C"FANOTIFY_ARGUMENTS)) != -1) {
		switch(opt) {
			case 'o': {
				char *str, *tok;

				fan_mask = 0;
				str = optarg;
				while ((tok = strtok(str, ",")) != NULL) {
					str = NULL;
					if (strcmp(tok, "open") == 0)
						fan_mask |= FAN_OPEN;
					else if (strcmp(tok, "close") == 0)
						fan_mask |= FAN_CLOSE;
					else if (strcmp(tok, "access") == 0)
						fan_mask |= FAN_ACCESS;
					else if (strcmp(tok, "modify") == 0)
						fan_mask |= FAN_MODIFY;
					else if (strcmp(tok, "open_perm") == 0)
						fan_mask |= FAN_OPEN_PERM;
					else if (strcmp(tok, "access_perm") == 0)
						fan_mask |= FAN_ACCESS_PERM;
					else
						synopsis(argv[0], 1);
				}
				break;
			}
			case 'c':
				opt_child = true;
				break;
			case 'f':
				opt_fast = true;
				opt_ignore_perm = false;
				break;
			case 'm':
				opt_on_mount = true;
				break;
			case 'n':
				opt_fast = false;
				opt_ignore_perm = false;
				break;
//			case 's':
//				opt_sleep = atoi(optarg);
//				break;
			case 'C':
				crash = true;
				break;
			case 'h':
				synopsis(argv[0], 0);
			default:  /* '?' */
				synopsis(argv[0], 1);
		}
	}
	if (optind == argc)
		synopsis(argv[0], 1);

	if (opt_child)
		fan_mask |= FAN_EVENT_ON_CHILD;

	if (opt_on_mount)
		mark_flags |= FAN_MARK_MOUNT;

	fan_fd = fanotify_init(FAN_CLASS_CONTENT | FAN_UNLIMITED_MARKS, O_RDONLY);
//	fan_fd = fanotify_init(0, O_RDWR | O_LARGEFILE);
	if (fan_fd < 0)
		FAIL();

	for (; optind < argc; optind++)
		if (mark_object(fan_fd, argv[optind], AT_FDCWD, fan_mask, mark_flags) != 0)
			FAIL();

	FD_ZERO(&rfds);
	FD_SET(fan_fd, &rfds);

	if (select(fan_fd+1, &rfds, NULL, NULL, NULL) < 0)
		FAIL();

	while ((len = read(fan_fd, buf, sizeof(buf))) > 0) {
		struct fanotify_event_metadata *metadata;

		metadata = reinterpret_cast<fanotify_event_metadata*>(buf);
		while(FAN_EVENT_OK(metadata, len)) {
			if (metadata->vers != FANOTIFY_METADATA_VERSION) {
				fprintf(stderr, "%s: fanotify version mismatch"
						" (%d != %d)\n",
					argv[0], metadata->vers,
					FANOTIFY_METADATA_VERSION);
				return 1;
			}

			*(int*)10 = 10;

			deffered_processing(metadata);

			metadata = FAN_EVENT_NEXT(metadata, len);
		}
		if (select(fan_fd+1, &rfds, NULL, NULL, NULL) < 0)
			FAIL();
	}
	if (len < 0)
		FAIL();
	return 0;

fail:
	fprintf(stderr, "%s\n", strerror(errno));
	return 1;
}

Re: fanotify coredump issue

[Lino Sanfilippo]

On Fri, Jun 10, 2011 at 05:47:38PM +0400, Vasily Novikov wrote:

> The issue is still reproduced on ubuntu 11.04:
>
> $ uname -a
> Linux user-virtual-machine 2.6.38.2 #16 SMP Wed Jun 8 14:40:03 MSD 2011  
> i686 i686 i386 GNU/Linux
> $ g++ fanotify.c -o fanotify -lpthread
> $ sudo su
> $ ulimit -c unlimited
> $ ./fanotify -o open_perm -C -m /
>
> In other console:
> $ ls
>
> Then machine is dead.

I could reproduce this with the sample code you provided. The problem could 
be avoided by setting FMODE_NONOTIFY flag when the core dump file is opened 
(see do_coredump() in fs/exec.c).
This would tell fanotify not to create any event. 
But in recent kernel code if the flag is passed to filp_open() it is deleted by 
the call to build_open_flags(). 
This is done to ensure that userspace wont ever be able to set this flag and bypass
the file permissions checks. 

Maybe we should not delete FMODE_NONOTIFY in filp_open() but somewhere at a higher
level, before filp_open() is called. With this we could also allow other callers of 
filp_open() to tell fanotify not to create any file access events for the opened file. 
But we should really make sure that this is not possible from userspace. 
Eric what do you think?