From patchwork Wed Jul 15 13:21:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jan Kara X-Patchwork-Id: 6798941 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id DF5FB9F2E8 for ; Wed, 15 Jul 2015 13:21:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 00494205DB for ; Wed, 15 Jul 2015 13:21:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B0647205D8 for ; Wed, 15 Jul 2015 13:21:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752598AbbGONVf (ORCPT ); Wed, 15 Jul 2015 09:21:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:47889 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752542AbbGONVb (ORCPT ); Wed, 15 Jul 2015 09:21:31 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 43CA1AC76; Wed, 15 Jul 2015 13:21:30 +0000 (UTC) Received: by quack.suse.cz (Postfix, from userid 1000) id 64EC882827; Wed, 15 Jul 2015 15:21:26 +0200 (CEST) From: Jan Kara To: Andrew Morton Cc: linux-fsdevel@vger.kernel.org, Jan Kara Subject: [PATCH 1/3] fsnotify: Fix oops in fsnotify_clear_marks_by_group_flags() Date: Wed, 15 Jul 2015 15:21:19 +0200 Message-Id: <1436966481-12517-2-git-send-email-jack@suse.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1436966481-12517-1-git-send-email-jack@suse.com> References: <1436966481-12517-1-git-send-email-jack@suse.com> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Jan Kara fsnotify_clear_marks_by_group_flags() can race with fsnotify_destroy_marks() so when fsnotify_destroy_mark_locked() drops mark_mutex, a mark from the list iterated by fsnotify_clear_marks_by_group_flags() can be freed and we dereference free memory in the loop there. Fix the problem by keeping mark_mutex held in fsnotify_destroy_mark_locked(). The reason why we drop that mutex is that we need to call a ->freeing_mark() callback which may acquire mark_mutex again. To avoid this and similar lock inversion issues, we move the call to ->freeing_mark() callback to the kthread destroying the mark. Reported-by: Ashish Sangwan Suggested-by: Lino Sanfilippo Signed-off-by: Jan Kara --- fs/notify/mark.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 92e48c70f0f0..3e594ce41010 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -152,31 +152,15 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, BUG(); list_del_init(&mark->g_list); - spin_unlock(&mark->lock); if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) iput(inode); - /* release lock temporarily */ - mutex_unlock(&group->mark_mutex); spin_lock(&destroy_lock); list_add(&mark->g_list, &destroy_list); spin_unlock(&destroy_lock); wake_up(&destroy_waitq); - /* - * We don't necessarily have a ref on mark from caller so the above destroy - * may have actually freed it, unless this group provides a 'freeing_mark' - * function which must be holding a reference. - */ - - /* - * Some groups like to know that marks are being freed. This is a - * callback to the group function to let it know that this mark - * is being freed. - */ - if (group->ops->freeing_mark) - group->ops->freeing_mark(mark, group); /* * __fsnotify_update_child_dentry_flags(inode); @@ -191,8 +175,6 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, */ atomic_dec(&group->num_marks); - - mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); } void fsnotify_destroy_mark(struct fsnotify_mark *mark, @@ -205,7 +187,10 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark, /* * Destroy all marks in the given list. The marks must be already detached from - * the original inode / vfsmount. + * the original inode / vfsmount. Note that we can race with + * fsnotify_clear_marks_by_group_flags(). However we hold a reference to each + * mark so they won't get freed from under us and nobody else touches our + * free_list list_head. */ void fsnotify_destroy_marks(struct list_head *to_free) { @@ -406,7 +391,7 @@ struct fsnotify_mark *fsnotify_find_mark(struct hlist_head *head, } /* - * clear any marks in a group in which mark->flags & flags is true + * Clear any marks in a group in which mark->flags & flags is true. */ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, unsigned int flags) @@ -460,6 +445,7 @@ static int fsnotify_mark_destroy(void *ignored) { struct fsnotify_mark *mark, *next; struct list_head private_destroy_list; + struct fsnotify_group *group; for (;;) { spin_lock(&destroy_lock); @@ -471,6 +457,14 @@ static int fsnotify_mark_destroy(void *ignored) list_for_each_entry_safe(mark, next, &private_destroy_list, g_list) { list_del_init(&mark->g_list); + group = mark->group; + /* + * Some groups like to know that marks are being freed. + * This is a callback to the group function to let it + * know that this mark is being freed. + */ + if (group && group->ops->freeing_mark) + group->ops->freeing_mark(mark, group); fsnotify_put_mark(mark); }