[PATCH 1/1] inotify: bug 77111 - fix reusage of watch descriptors

[Heinrich Schuchardt]

Without the patch inotify watch descriptors may be reused by
inotify_add_watch before all events for the previous usage
of the watch descriptor have been read.

With the patch watch descriptors are removed from the idr only
after the IN_IGNORED event has been read.

The sequence of some static routines is rearranged.

The significant change moving the call of
inotify_remove_from_idr form inotify_ignored_and_remove_idr to
to copy_event_to_user and renaming inotify_ignored_and_remove_idr
to inotify_ignored.

cf.
https://bugzilla.kernel.org/show_bug.cgi?id=77111

Signed-off-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
---
 fs/notify/inotify/inotify.h          |   4 +-
 fs/notify/inotify/inotify_fsnotify.c |   2 +-
 fs/notify/inotify/inotify_user.c     | 257 ++++++++++++++++++-----------------
 3 files changed, 135 insertions(+), 128 deletions(-)

diff --git a/fs/notify/inotify/inotify.h b/fs/notify/inotify/inotify.h
index ed855ef..596c513 100644
--- a/fs/notify/inotify/inotify.h
+++ b/fs/notify/inotify/inotify.h
@@ -20,8 +20,8 @@ static inline struct inotify_event_info *INOTIFY_E(struct fsnotify_event *fse)
 	return container_of(fse, struct inotify_event_info, fse);
 }
 
-extern void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
-					   struct fsnotify_group *group);
+extern void inotify_ignored(struct fsnotify_mark *fsn_mark,
+			    struct fsnotify_group *group);
 extern int inotify_handle_event(struct fsnotify_group *group,
 				struct inode *inode,
 				struct fsnotify_mark *inode_mark,
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index 43ab1e1..68729dd 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -122,7 +122,7 @@ int inotify_handle_event(struct fsnotify_group *group,
 
 static void inotify_freeing_mark(struct fsnotify_mark *fsn_mark, struct fsnotify_group *group)
 {
-	inotify_ignored_and_remove_idr(fsn_mark, group);
+	inotify_ignored(fsn_mark, group);
 }
 
 /*
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 78a2ca3..7a354b0 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -164,115 +164,6 @@ static struct fsnotify_event *get_one_event(struct fsnotify_group *group,
 	return event;
 }
 
-/*
- * Copy an event to user space, returning how much we copied.
- *
- * We already checked that the event size is smaller than the
- * buffer we had in "get_one_event()" above.
- */
-static ssize_t copy_event_to_user(struct fsnotify_group *group,
-				  struct fsnotify_event *fsn_event,
-				  char __user *buf)
-{
-	struct inotify_event inotify_event;
-	struct inotify_event_info *event;
-	size_t event_size = sizeof(struct inotify_event);
-	size_t name_len;
-	size_t pad_name_len;
-
-	pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
-
-	event = INOTIFY_E(fsn_event);
-	name_len = event->name_len;
-	/*
-	 * round up name length so it is a multiple of event_size
-	 * plus an extra byte for the terminating '\0'.
-	 */
-	pad_name_len = round_event_name_len(fsn_event);
-	inotify_event.len = pad_name_len;
-	inotify_event.mask = inotify_mask_to_arg(fsn_event->mask);
-	inotify_event.wd = event->wd;
-	inotify_event.cookie = event->sync_cookie;
-
-	/* send the main event */
-	if (copy_to_user(buf, &inotify_event, event_size))
-		return -EFAULT;
-
-	buf += event_size;
-
-	/*
-	 * fsnotify only stores the pathname, so here we have to send the pathname
-	 * and then pad that pathname out to a multiple of sizeof(inotify_event)
-	 * with zeros.
-	 */
-	if (pad_name_len) {
-		/* copy the path name */
-		if (copy_to_user(buf, event->name, name_len))
-			return -EFAULT;
-		buf += name_len;
-
-		/* fill userspace with 0's */
-		if (clear_user(buf, pad_name_len - name_len))
-			return -EFAULT;
-		event_size += pad_name_len;
-	}
-
-	return event_size;
-}
-
-static ssize_t inotify_read(struct file *file, char __user *buf,
-			    size_t count, loff_t *pos)
-{
-	struct fsnotify_group *group;
-	struct fsnotify_event *kevent;
-	char __user *start;
-	int ret;
-	DEFINE_WAIT(wait);
-
-	start = buf;
-	group = file->private_data;
-
-	while (1) {
-		prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
-
-		mutex_lock(&group->notification_mutex);
-		kevent = get_one_event(group, count);
-		mutex_unlock(&group->notification_mutex);
-
-		pr_debug("%s: group=%p kevent=%p\n", __func__, group, kevent);
-
-		if (kevent) {
-			ret = PTR_ERR(kevent);
-			if (IS_ERR(kevent))
-				break;
-			ret = copy_event_to_user(group, kevent, buf);
-			fsnotify_destroy_event(group, kevent);
-			if (ret < 0)
-				break;
-			buf += ret;
-			count -= ret;
-			continue;
-		}
-
-		ret = -EAGAIN;
-		if (file->f_flags & O_NONBLOCK)
-			break;
-		ret = -ERESTARTSYS;
-		if (signal_pending(current))
-			break;
-
-		if (start != buf)
-			break;
-
-		schedule();
-	}
-
-	finish_wait(&group->notification_waitq, &wait);
-	if (start != buf && ret != -EFAULT)
-		ret = buf - start;
-	return ret;
-}
-
 static int inotify_release(struct inode *ignored, struct file *file)
 {
 	struct fsnotify_group *group = file->private_data;
@@ -315,18 +206,6 @@ static long inotify_ioctl(struct file *file, unsigned int cmd,
 	return ret;
 }
 
-static const struct file_operations inotify_fops = {
-	.show_fdinfo	= inotify_show_fdinfo,
-	.poll		= inotify_poll,
-	.read		= inotify_read,
-	.fasync		= fsnotify_fasync,
-	.release	= inotify_release,
-	.unlocked_ioctl	= inotify_ioctl,
-	.compat_ioctl	= inotify_ioctl,
-	.llseek		= noop_llseek,
-};
-
-
 /*
  * find_inode - resolve a user-given path to a specific inode
  */
@@ -488,8 +367,8 @@ out:
 /*
  * Send IN_IGNORED for this wd, remove this wd from the idr.
  */
-void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
-				    struct fsnotify_group *group)
+void inotify_ignored(struct fsnotify_mark *fsn_mark,
+		     struct fsnotify_group *group)
 {
 	struct inotify_inode_mark *i_mark;
 
@@ -498,8 +377,6 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark *fsn_mark,
 			     NULL, FSNOTIFY_EVENT_NONE, NULL, 0);
 
 	i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark);
-	/* remove this mark from the idr */
-	inotify_remove_from_idr(group, i_mark);
 
 	atomic_dec(&group->inotify_data.user->inotify_watches);
 }
@@ -665,6 +542,136 @@ static struct fsnotify_group *inotify_new_group(unsigned int max_events)
 	return group;
 }
 
+/*
+ * Copy an event to user space, returning how much we copied.
+ *
+ * We already checked that the event size is smaller than the
+ * buffer we had in "get_one_event()" above.
+ */
+static ssize_t copy_event_to_user(struct fsnotify_group *group,
+				  struct fsnotify_event *fsn_event,
+				  char __user *buf)
+{
+	struct inotify_event inotify_event;
+	struct inotify_event_info *event;
+	struct inotify_inode_mark *found_i_mark = NULL;
+	size_t event_size = sizeof(struct inotify_event);
+	size_t name_len;
+	size_t pad_name_len;
+
+	pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
+
+	event = INOTIFY_E(fsn_event);
+	name_len = event->name_len;
+	/*
+	 * round up name length so it is a multiple of event_size
+	 * plus an extra byte for the terminating '\0'.
+	 */
+	pad_name_len = round_event_name_len(fsn_event);
+	inotify_event.len = pad_name_len;
+	inotify_event.mask = inotify_mask_to_arg(fsn_event->mask);
+	inotify_event.wd = event->wd;
+	inotify_event.cookie = event->sync_cookie;
+
+	/* send the main event */
+	if (copy_to_user(buf, &inotify_event, event_size))
+		return -EFAULT;
+
+	if (inotify_event.mask & IN_IGNORED) {
+		found_i_mark = inotify_idr_find(group, inotify_event.wd);
+		if (found_i_mark) {
+			/* remove this mark from the idr */
+			inotify_remove_from_idr(group, found_i_mark);
+			/* match ref taken by inotify_idr_find */
+			fsnotify_put_mark(&found_i_mark->fsn_mark);
+		}
+	}
+
+	buf += event_size;
+
+	/*
+	 * fsnotify only stores the pathname, so here we have to send the pathname
+	 * and then pad that pathname out to a multiple of sizeof(inotify_event)
+	 * with zeros.
+	 */
+	if (pad_name_len) {
+		/* copy the path name */
+		if (copy_to_user(buf, event->name, name_len))
+			return -EFAULT;
+		buf += name_len;
+
+		/* fill userspace with 0's */
+		if (clear_user(buf, pad_name_len - name_len))
+			return -EFAULT;
+		event_size += pad_name_len;
+	}
+
+	return event_size;
+}
+
+static ssize_t inotify_read(struct file *file, char __user *buf,
+			    size_t count, loff_t *pos)
+{
+	struct fsnotify_group *group;
+	struct fsnotify_event *kevent;
+	char __user *start;
+	int ret;
+	DEFINE_WAIT(wait);
+
+	start = buf;
+	group = file->private_data;
+
+	while (1) {
+		prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
+
+		mutex_lock(&group->notification_mutex);
+		kevent = get_one_event(group, count);
+		mutex_unlock(&group->notification_mutex);
+
+		pr_debug("%s: group=%p kevent=%p\n", __func__, group, kevent);
+
+		if (kevent) {
+			ret = PTR_ERR(kevent);
+			if (IS_ERR(kevent))
+				break;
+			ret = copy_event_to_user(group, kevent, buf);
+			fsnotify_destroy_event(group, kevent);
+			if (ret < 0)
+				break;
+			buf += ret;
+			count -= ret;
+			continue;
+		}
+
+		ret = -EAGAIN;
+		if (file->f_flags & O_NONBLOCK)
+			break;
+		ret = -ERESTARTSYS;
+		if (signal_pending(current))
+			break;
+
+		if (start != buf)
+			break;
+
+		schedule();
+	}
+
+	finish_wait(&group->notification_waitq, &wait);
+	if (start != buf && ret != -EFAULT)
+		ret = buf - start;
+	return ret;
+}
+
+static const struct file_operations inotify_fops = {
+	.show_fdinfo	= inotify_show_fdinfo,
+	.poll		= inotify_poll,
+	.read		= inotify_read,
+	.fasync		= fsnotify_fasync,
+	.release	= inotify_release,
+	.unlocked_ioctl	= inotify_ioctl,
+	.compat_ioctl	= inotify_ioctl,
+	.llseek		= noop_llseek,
+};
 
 /* inotify syscalls */
 SYSCALL_DEFINE1(inotify_init1, int, flags)
-- 
2.0.0.rc2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/