[patch]A potential bug in inotify_user.c

[patch]A potential bug in inotify_user.c

[Yan Zheng]

Hello,

follow comment is at fs/inotify_user.c:287
/* coalescing: drop this event if it is a dupe of the previous */

I think the previous event in the comment should be the last event in
the link list. But inotify_dev_get_event return the first event in the
list. In addition, it doesn't check whether the list is empty

Regards

Signed-off-by: Yan Zheng<yanzheng@21cn.com>
---
diff -ur linux-2.6.23-rc8/fs/inotify_user.c linux/fs/inotify_user.c
--- linux-2.6.23-rc8/fs/inotify_user.c	2007-09-29 11:00:15.000000000 +0800
+++ linux/fs/inotify_user.c	2007-09-29 11:01:40.000000000 +0800
@@ -247,6 +247,19 @@
 }

 /*
+ * inotify_dev_get_last_event - return the last event in the given dev's queue
+ *
+ * Caller must hold dev->ev_mutex.
+ */
+static inline struct inotify_kernel_event *
+inotify_dev_get_last_event(struct inotify_device *dev)
+{
+	if (list_empty(&dev->events))
+		return NULL;
+	return list_entry(dev->events.prev, struct inotify_kernel_event, list);
+}
+
+/*
  * inotify_dev_queue_event - event handler registered with core inotify, adds
  * a new event to the given device
  *
@@ -272,7 +285,7 @@
 		put_inotify_watch(w); /* final put */

 	/* coalescing: drop this event if it is a dupe of the previous */
-	last = inotify_dev_get_event(dev);
+	last = inotify_dev_get_last_event(dev);
 	if (last && last->event.mask == mask && last->event.wd == wd &&
 			last->event.cookie == cookie) {
 		const char *lastname = last->name;

Re: [patch]A potential bug in inotify_user.c

[Andrew Morton]

On Sat, 29 Sep 2007 11:28:22 +0800
"Yan Zheng" <yanzheng@21cn.com> wrote:

> Hello,
> 
> follow comment is at fs/inotify_user.c:287
> /* coalescing: drop this event if it is a dupe of the previous */
> 
> I think the previous event in the comment should be the last event in
> the link list. But inotify_dev_get_event return the first event in the
> list. In addition, it doesn't check whether the list is empty
> 
> Regards
> 
> Signed-off-by: Yan Zheng<yanzheng@21cn.com>
> ---
> diff -ur linux-2.6.23-rc8/fs/inotify_user.c linux/fs/inotify_user.c
> --- linux-2.6.23-rc8/fs/inotify_user.c	2007-09-29 11:00:15.000000000 +0800
> +++ linux/fs/inotify_user.c	2007-09-29 11:01:40.000000000 +0800
> @@ -247,6 +247,19 @@
>  }
> 
>  /*
> + * inotify_dev_get_last_event - return the last event in the given dev's queue
> + *
> + * Caller must hold dev->ev_mutex.
> + */
> +static inline struct inotify_kernel_event *
> +inotify_dev_get_last_event(struct inotify_device *dev)
> +{
> +	if (list_empty(&dev->events))
> +		return NULL;
> +	return list_entry(dev->events.prev, struct inotify_kernel_event, list);
> +}
> +
> +/*
>   * inotify_dev_queue_event - event handler registered with core inotify, adds
>   * a new event to the given device
>   *
> @@ -272,7 +285,7 @@
>  		put_inotify_watch(w); /* final put */
> 
>  	/* coalescing: drop this event if it is a dupe of the previous */
> -	last = inotify_dev_get_event(dev);
> +	last = inotify_dev_get_last_event(dev);
>  	if (last && last->event.mask == mask && last->event.wd == wd &&
>  			last->event.cookie == cookie) {
>  		const char *lastname = last->name;

So this has been floating around in my to-look-at queue for a month. 
Probably Robert didn't see it.

Could we take a look at it please?  

Re: [patch]A potential bug in inotify_user.c

[Robert Love]

On 10/22/07, Andrew Morton <akpm@linux-foundation.org> wrote:

> So this has been floating around in my to-look-at queue for a month.
> Probably Robert didn't see it.
>
> Could we take a look at it please?

Looks right to me, although I'm surprised no one else has noticed this.

Cutch, you confirm?

Acked-by: Robert Love <rlove@rlove.org>

     Robert