mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/3] notify: unused event private race
@ 2009-08-17  1:51 Eric Paris
  2009-08-17  1:51 ` [PATCH 2/3] inotify: tail drop inotify q_overflow events Eric Paris
  2009-08-17  1:51 ` [PATCH 3/3] inotify: start watch descriptor count at 1 Eric Paris
  0 siblings, 2 replies; 3+ messages in thread
From: Eric Paris @ 2009-08-17  1:51 UTC (permalink / raw)
  To: linux-kernel, fs-devel; +Cc: torvalds, viro

inotify decides if private data it passed to get added to an event was used
by checking list_empty().  But it's possible that the event may have been
dequeued and the private event removed so it would look empty.  The fix is
to use the return code from fsnotify_add_notify_event rather than looking
at the list.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/inotify/inotify_fsnotify.c |   13 +++++++------
 fs/notify/inotify/inotify_user.c     |    7 +++----
 fs/notify/notification.c             |    7 +++----
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index 47cd258..5dcbafe 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -62,13 +62,14 @@ static int inotify_handle_event(struct fsnotify_group *group, struct fsnotify_ev
 	event_priv->wd = wd;
 
 	ret = fsnotify_add_notify_event(group, event, fsn_event_priv);
-	/* EEXIST is not an error */
-	if (ret == -EEXIST)
-		ret = 0;
-
-	/* did event_priv get attached? */
-	if (list_empty(&fsn_event_priv->event_list))
+	if (ret) {
 		inotify_free_event_priv(fsn_event_priv);
+		/* EEXIST says we tail matched, EOVERFLOW isn't something
+		 * to report up the stack. */
+		if ((ret == -EEXIST) ||
+		    (ret == -EOVERFLOW))
+			ret = 0;
+	}
 
 	/*
 	 * If we hold the entry until after the event is on the queue
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index c17d5a3..54cbded 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -386,6 +386,7 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark_entry *entry,
 	struct fsnotify_event *ignored_event;
 	struct inotify_event_private_data *event_priv;
 	struct fsnotify_event_private_data *fsn_event_priv;
+	int ret;
 
 	ignored_event = fsnotify_create_event(NULL, FS_IN_IGNORED, NULL,
 					      FSNOTIFY_EVENT_NONE, NULL, 0,
@@ -404,10 +405,8 @@ void inotify_ignored_and_remove_idr(struct fsnotify_mark_entry *entry,
 	fsn_event_priv->group = group;
 	event_priv->wd = ientry->wd;
 
-	fsnotify_add_notify_event(group, ignored_event, fsn_event_priv);
-
-	/* did the private data get added? */
-	if (list_empty(&fsn_event_priv->event_list))
+	ret = fsnotify_add_notify_event(group, ignored_event, fsn_event_priv);
+	if (ret)
 		inotify_free_event_priv(fsn_event_priv);
 
 skip_send_ignore:
diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 5213685..74b3cf3 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -171,9 +171,7 @@ int fsnotify_add_notify_event(struct fsnotify_group *group, struct fsnotify_even
 	struct list_head *list = &group->notification_list;
 	struct fsnotify_event_holder *last_holder;
 	struct fsnotify_event *last_event;
-
-	/* easy to tell if priv was attached to the event */
-	INIT_LIST_HEAD(&priv->event_list);
+	int ret = 0;
 
 	/*
 	 * There is one fsnotify_event_holder embedded inside each fsnotify_event.
@@ -194,6 +192,7 @@ alloc_holder:
 
 	if (group->q_len >= group->max_events) {
 		event = &q_overflow_event;
+		ret = -EOVERFLOW;
 		/* sorry, no private data on the overflow event */
 		priv = NULL;
 	}
@@ -235,7 +234,7 @@ alloc_holder:
 	mutex_unlock(&group->notification_mutex);
 
 	wake_up(&group->notification_waitq);
-	return 0;
+	return ret;
 }
 
 /*


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/3] inotify: tail drop inotify q_overflow events
  2009-08-17  1:51 [PATCH 1/3] notify: unused event private race Eric Paris
@ 2009-08-17  1:51 ` Eric Paris
  2009-08-17  1:51 ` [PATCH 3/3] inotify: start watch descriptor count at 1 Eric Paris
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Paris @ 2009-08-17  1:51 UTC (permalink / raw)
  To: linux-kernel, fs-devel; +Cc: torvalds, viro

In f44aebcc the tail drop logic of events with no file backing
(q_overflow and in_ignored) was reversed so IN_IGNORED events would never
be tail dropped.  This now means that Q_OVERFLOW events are NOT tail
dropped.  The fix is to not tail drop IN_IGNORED, but to tail drop
Q_OVERFLOW.

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/notification.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/notify/notification.c b/fs/notify/notification.c
index 74b3cf3..3816d57 100644
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -153,6 +153,10 @@ static bool event_compare(struct fsnotify_event *old, struct fsnotify_event *new
 				return true;
 			break;
 		case (FSNOTIFY_EVENT_NONE):
+			if (old->mask & FS_Q_OVERFLOW)
+				return true;
+			else if (old->mask & FS_IN_IGNORED)
+				return false;
 			return false;
 		};
 	}


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 3/3] inotify: start watch descriptor count at 1
  2009-08-17  1:51 [PATCH 1/3] notify: unused event private race Eric Paris
  2009-08-17  1:51 ` [PATCH 2/3] inotify: tail drop inotify q_overflow events Eric Paris
@ 2009-08-17  1:51 ` Eric Paris
  1 sibling, 0 replies; 3+ messages in thread
From: Eric Paris @ 2009-08-17  1:51 UTC (permalink / raw)
  To: linux-kernel, fs-devel; +Cc: torvalds, viro

The inotify_add_watch man page specifies that inotify_add_watch() will
return a non-negative integer.  However, historically the inotify watches
started at 1, not at 0.  Turns out that the inotifywait program provided by
the inotify-tools package doesn't properly handle a 0 watch descriptor.
In 7e790dd5 we changed from starting at 1 to starting at 0.  This patch
starts at 1, just like in previous kernels, but also just like in previous
kernels it's possible for it to wrap back to 0.  This preserves the kernel
functionality exactly like it was before the patch (neither method broke
the spec)

Signed-off-by: Eric Paris <eparis@redhat.com>
---

 fs/notify/inotify/inotify_user.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 54cbded..ca0f36b 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -567,7 +567,7 @@ static struct fsnotify_group *inotify_new_group(struct user_struct *user, unsign
 
 	spin_lock_init(&group->inotify_data.idr_lock);
 	idr_init(&group->inotify_data.idr);
-	group->inotify_data.last_wd = 0;
+	group->inotify_data.last_wd = 1;
 	group->inotify_data.user = user;
 	group->inotify_data.fa = NULL;
 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-08-17  1:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-17  1:51 [PATCH 1/3] notify: unused event private race Eric Paris
2009-08-17  1:51 ` [PATCH 2/3] inotify: tail drop inotify q_overflow events Eric Paris
2009-08-17  1:51 ` [PATCH 3/3] inotify: start watch descriptor count at 1 Eric Paris

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®