2010-11-09 17:19:53

by Lino Sanfilippo

Subject: [PATCH] fanotify: correct broken ref counting in case adding a mark failed

If adding a mount or inode mark failed fanotify_free_mark() is called explicitly.
But at this time the mark has already been put into the destroy list of the
fsnotify_mark kernel thread. If the thread is too slow it will try to decrease
the reference of a mark, that has already been freed by fanotify_free_mark().
(If its fast enough it will only decrease the marks ref counter from 2 to 1 - note
that the counter has been increased to 2 in add_mark() - which has practically no
effect.)
This patch fixes the ref counting by not calling free_mark() explicitly, but
decreasing the ref counter and rely on the fsnotify_mark thread to cleanup in
case adding the mark has failed.

Signed-off-by: Lino Sanfilippo <[email protected]>
---
This patch applies against commit 02478ec882e057e33fbdd8f1ba245fdfa5a355fb
of branch 'origin/for-next' from git.infradead.org/users/eparis/notify.git

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index a7a9d84..090790a 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -605,11 +605,10 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,
{
struct fsnotify_mark *fsn_mark;
__u32 added;
+ int ret = 0;

fsn_mark = fsnotify_find_vfsmount_mark(group, mnt);
if (!fsn_mark) {
- int ret;
-
if (atomic_read(&group->num_marks) > group->fanotify_data.max_marks)
return -ENOSPC;

@@ -619,17 +618,16 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,

fsnotify_init_mark(fsn_mark, fanotify_free_mark);
ret = fsnotify_add_mark(fsn_mark, group, NULL, mnt, 0);
- if (ret) {
- fanotify_free_mark(fsn_mark);
- return ret;
- }
+ if (ret)
+ goto err;
}
added = fanotify_mark_add_to_mask(fsn_mark, mask, flags);
- fsnotify_put_mark(fsn_mark);
+
if (added & ~mnt->mnt_fsnotify_mask)
fsnotify_recalc_vfsmount_mask(mnt);
-
- return 0;
+err:
+ fsnotify_put_mark(fsn_mark);
+ return ret;
}

static int fanotify_add_inode_mark(struct fsnotify_group *group,
@@ -638,6 +636,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
{
struct fsnotify_mark *fsn_mark;
__u32 added;
+ int ret = 0;

pr_debug("%s: group=%p inode=%p\n", __func__, group, inode);

@@ -653,8 +652,6 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,

fsn_mark = fsnotify_find_inode_mark(group, inode);
if (!fsn_mark) {
- int ret;
-
if (atomic_read(&group->num_marks) > group->fanotify_data.max_marks)
return -ENOSPC;

@@ -664,16 +661,16 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,

fsnotify_init_mark(fsn_mark, fanotify_free_mark);
ret = fsnotify_add_mark(fsn_mark, group, inode, NULL, 0);
- if (ret) {
- fanotify_free_mark(fsn_mark);
- return ret;
- }
+ if (ret)
+ goto err;
}
added = fanotify_mark_add_to_mask(fsn_mark, mask, flags);
- fsnotify_put_mark(fsn_mark);
+
if (added & ~inode->i_fsnotify_mask)
fsnotify_recalc_inode_mask(inode);
- return 0;
+err:
+ fsnotify_put_mark(fsn_mark);
+ return ret;
}

/* fanotify syscalls */

2010-11-09 20:21:17

by Eric Paris

Subject: Re: [PATCH] fanotify: correct broken ref counting in case adding a mark failed

On Tue, 2010-11-09 at 18:18 +0100, Lino Sanfilippo wrote:
> If adding a mount or inode mark failed fanotify_free_mark() is called explicitly.
> But at this time the mark has already been put into the destroy list of the
> fsnotify_mark kernel thread. If the thread is too slow it will try to decrease
> the reference of a mark, that has already been freed by fanotify_free_mark().
> (If its fast enough it will only decrease the marks ref counter from 2 to 1 - note
> that the counter has been increased to 2 in add_mark() - which has practically no
> effect.)
> This patch fixes the ref counting by not calling free_mark() explicitly, but
> decreasing the ref counter and rely on the fsnotify_mark thread to cleanup in
> case adding the mark has failed.
>
> Signed-off-by: Lino Sanfilippo <[email protected]>

applied to http://git.infradead.org/users/eparis/notify.git/ #for-next

-Eric