Skip to contentSkip to conversation

fs, fanotify: dont send notification event for coredump file

PATCH In review Lino Sanfilippo 2 participants 3 messages Mar 25, 2012 – Mar 26, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fs, fanotify: dont send notification event for coredump file

Currently fsnotify events are created for core dump files. This may lead to problems if a listener has registered with fanotify to watch permission events for the whole filesystem:
If it is the listener itself that causes the creation of a core dump file (due to a segfault) a permission event is created but never granted (the listener is not running any more).
OTOH the listener cant unregister either, since its waiting for its own ack. Since no file accesses are answered any more, eventually the whole system freezes.

This patch solves the problem by allowing the core dump file to be created without sending a notification event. For this reason the FMODE_NONOTIFY flag is no longer cleared in build_open_flags() but in the caller. Furthermore it is set for the creation of the core dump file.
Also callers of filp_open() are now able to specify FMODE_NONOTIFY if they dont want notification events to be created for an opened file.

---
s patch applies against 3.3
The bug has been reported here:
http://marc.info/?l=linux-fsdevel&m=129346873312523&w=2

 fs/exec.c |    5 ++---
 fs/open.c |    9 ++++-----
 2 files changed, 6 insertions(+), 8 deletions(-)
fs/exec.c +2 -3
index 153dee1..3c65946 100644
@@ -2197,9 +2197,8 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 		if (cprm.limit < binfmt->min_coredump)
 			goto fail_unlock;
 
-		cprm.file = filp_open(cn.corename,
-				 O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag,
-				 0600);
+		flag |= O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | FMODE_NONOTIFY;
+		cprm.file = filp_open(cn.corename, flag, 0600);
 		if (IS_ERR(cprm.file))
 			goto fail_unlock;
 
fs/open.c +4 -5
index 77becc0..955fe2f 100644
@@ -886,9 +886,6 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
 		mode = 0;
 	op->mode = mode;
 
-	/* Must never be set by userspace */
-	flags &= ~FMODE_NONOTIFY;
-
 	/*
 	 * O_SYNC is implemented as __O_SYNC|O_DSYNC.  As many places only
 	 * check for O_DSYNC if the need any syncing at all we enforce it's
@@ -947,6 +944,8 @@ static inline int build_open_flags(int flags, umode_t mode, struct open_flags *o
  * This is the helper to open a file from kernelspace if you really
  * have to.  But in generally you should not do this, so please move
  * along, nothing to see here..
+ * NOTE: If flags are given from userspace consider to ensure that
+ * FMODE_NONOTIFY is not set.
  */
 struct file *filp_open(const char *filename, int flags, umode_t mode)
 {
@@ -960,7 +959,7 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 			    const char *filename, int flags)
 {
 	struct open_flags op;
-	int lookup = build_open_flags(flags, 0, &op);
+	int lookup = build_open_flags(flags & ~FMODE_NONOTIFY, 0, &op);
 	if (flags & O_CREAT)
 		return ERR_PTR(-EINVAL);
 	if (!filename && (flags & O_DIRECTORY))
@@ -973,7 +972,7 @@ EXPORT_SYMBOL(file_open_root);
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
 	struct open_flags op;
-	int lookup = build_open_flags(flags, mode, &op);
+	int lookup = build_open_flags(flags & ~FMODE_NONOTIFY, mode, &op);
 	char *tmp = getname(filename);
 	int fd = PTR_ERR(tmp);
 
Signature · 1 line
1.7.2.5
#1 lore raw
Quoted 18 lines · Lino Sanfilippo

Currently fsnotify events are created for core dump files. This may lead to problems if a listener has registered with fanotify to watch permission events for the whole filesystem:
If it is the listener itself that causes the creation of a core dump file (due to a segfault) a permission event is created but never granted (the listener is not running any more).
OTOH the listener cant unregister either, since its waiting for its own ack. Since no file accesses are answered any more, eventually the whole system freezes.

This patch solves the problem by allowing the core dump file to be created without sending a notification event. For this reason the FMODE_NONOTIFY flag is no longer cleared in build_open_flags() but in the caller. Furthermore it is set for the creation of the core dump file.
Also callers of filp_open() are now able to specify FMODE_NONOTIFY if they dont want notification events to be created for an opened file.

Quoted 2 lines

How does this work with something like abrt, which causes core files to be created in a known dir. It then watches (I have no idea if it uses *notify) for new core files and processes them when they appear. Would we be able to get any notification of core files with this patch? Maybe we need a way to just not allow ourselves to block on core files?

#2 lore raw
Quoted 5 lines · Eric Paris

How does this work with something like abrt, which causes core files to be created in a known dir. It then watches (I have no idea if it uses *notify) for new core files and processes them when they appear. Would we be able to get any notification of core files with this patch? Maybe we need a way to just not allow ourselves to block on core files?

No we would not get notifications for core files with this patch any more. I see that this is a drawback for applications that want to track those files.
But unfortunately I dont see how we could tell if a core file is created by a fanotify listener or not.

The easiest thing would probably be if we had two flags, one to suppress "pure notification" events and one to suppress permission event.

For example if we had:
FMODE_NONOTIFY: suppress pure notification events
FMODE_NOPERMNOTIFY: suppress permission events

we would only use FMODE_NOPERMNOTIFY for core files then.