Re: [PATCH 2/2] fsnotify: Protect bail out path of fsnotify_add_mark_locked() properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 31, 2017 at 11:33 AM, Jan Kara <jack@xxxxxxx> wrote:
> When fsnotify_add_mark_locked() fails it cleans up the mark it was
> adding. Since the mark is already visible in group's list, we should
> protect update of mark->flags with mark->lock. I'm not aware of any real
> issues this could cause (since we also hold group->mark_mutex) but
> better be safe and obey locking rules properly.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>

Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>

IMO, even though this does not fix a concrete bug, if it's worth
fixing in upstream, it's worth fixing in stable.
A future stable fix may either make this into a concrete bug
or just be harder to apply.

So I suggest to add the Fixes: and Cc: stable tags.

Greg,

Do you agree with this reasoning?


> ---
>  fs/notify/mark.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 9991f8826734..47a827975b58 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -599,9 +599,11 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark, struct inode *inode,
>
>         return ret;
>  err:
> +       spin_lock(&mark->lock);
>         mark->flags &= ~(FSNOTIFY_MARK_FLAG_ALIVE |
>                          FSNOTIFY_MARK_FLAG_ATTACHED);
>         list_del_init(&mark->g_list);
> +       spin_unlock(&mark->lock);
>         atomic_dec(&group->num_marks);
>
>         fsnotify_put_mark(mark);
> --
> 2.12.3
>



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux