Skip to contentSkip to conversation

fsnotify: potential use after free

PATCH In review Dan Carpenter 2 participants 2 messages Jul 22, 2013

Subsystems (2)
Kernel janitors Fsnotify: filesystem notification infrastructure
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 fsnotify: potential use after free

My static checker complains that if we drop the last reference then it would be a use after free. I don't know if it's possible, but really the atomic_dec(&group->num_marks); should be done while we are holding a reference to "group".

Signed-off-by Dan Carpenter <[email protected]>
fs/notify/mark.c +1 -1
index 923fe4a..27e357e 100644
@@ -262,9 +262,9 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 err:
 	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
 	list_del_init(&mark->g_list);
+	atomic_dec(&group->num_marks);
 	fsnotify_put_group(group);
 	mark->group = NULL;
-	atomic_dec(&group->num_marks);
 
 	spin_unlock(&mark->lock);
#1 lore raw
Quoted 13 lines · Dan Carpenter

My static checker complains that if we drop the last reference then it would be a use after free. I don't know if it's possible, but really the atomic_dec(&group->num_marks); should be done while we are holding a reference to "group".

Signed-off-by Dan Carpenter <[email protected]>
fs/notify/mark.c +0 -0
index 923fe4a..27e357e 100644

fs/notify/mark.c int fsnotify_add_mark_locked(struct fsnotify_mark *mark, · line 270

Quoted 10 lines
fs/notify/mark.c +1 -1
@@ -262,9 +262,9 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
  err:
  	mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE;
  	list_del_init(&mark->g_list);
+	atomic_dec(&group->num_marks);
  	fsnotify_put_group(group);
  	mark->group = NULL;
-	atomic_dec(&group->num_marks);
 
  	spin_unlock(&mark->lock);

Hi Dan,

we assume that the caller already holds a reference to a group. We then take another ref for the marks->group. This is the one that we drop in case of error, but there is still the one held by the caller. So its still save to access the group and to decrement the number of marks since we always hold a ref to it.
However, thank you for testing.

Regards,
Lino