stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Miklos Szeredi <mszeredi@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [PATCH 4.7 16/69] fanotify: fix list corruption in fanotify_get_response()
Date: Wed, 28 Sep 2016 11:04:58 +0200	[thread overview]
Message-ID: <20160928090445.779296493@linuxfoundation.org> (raw)
In-Reply-To: <20160928090445.054716307@linuxfoundation.org>

4.7-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Jan Kara <jack@suse.cz>

commit 96d41019e3ac55f6f0115b0ce97e4f24a3d636d2 upstream.

fanotify_get_response() calls fsnotify_remove_event() when it finds that
group is being released from fanotify_release() (bypass_perm is set).

However the event it removes need not be only in the group's notification
queue but it can have already moved to access_list (userspace read the
event before closing the fanotify instance fd) which is protected by a
different lock.  Thus when fsnotify_remove_event() races with
fanotify_release() operating on access_list, the list can get corrupted.

Fix the problem by moving all the logic removing permission events from
the lists to one place - fanotify_release().

Fixes: 5838d4442bd5 ("fanotify: fix double free of pending permission events")
Link: http://lkml.kernel.org/r/1473797711-14111-3-git-send-email-jack@suse.cz
Signed-off-by: Jan Kara <jack@suse.cz>
Reported-by: Miklos Szeredi <mszeredi@redhat.com>
Tested-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 fs/notify/fanotify/fanotify.c      |   13 +------------
 fs/notify/fanotify/fanotify_user.c |   36 ++++++++++++++++++++++++------------
 fs/notify/notification.c           |   15 ---------------
 include/linux/fsnotify_backend.h   |    3 ---
 4 files changed, 25 insertions(+), 42 deletions(-)

--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -67,18 +67,7 @@ static int fanotify_get_response(struct
 
 	pr_debug("%s: group=%p event=%p\n", __func__, group, event);
 
-	wait_event(group->fanotify_data.access_waitq, event->response ||
-				atomic_read(&group->fanotify_data.bypass_perm));
-
-	if (!event->response) {	/* bypass_perm set */
-		/*
-		 * Event was canceled because group is being destroyed. Remove
-		 * it from group's event list because we are responsible for
-		 * freeing the permission event.
-		 */
-		fsnotify_remove_event(group, &event->fae.fse);
-		return 0;
-	}
+	wait_event(group->fanotify_data.access_waitq, event->response);
 
 	/* userspace responded, convert to something usable */
 	switch (event->response) {
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -358,16 +358,20 @@ static int fanotify_release(struct inode
 
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
 	struct fanotify_perm_event_info *event, *next;
+	struct fsnotify_event *fsn_event;
 
 	/*
-	 * There may be still new events arriving in the notification queue
-	 * but since userspace cannot use fanotify fd anymore, no event can
-	 * enter or leave access_list by now.
+	 * Stop new events from arriving in the notification queue. since
+	 * userspace cannot use fanotify fd anymore, no event can enter or
+	 * leave access_list by now either.
 	 */
-	spin_lock(&group->fanotify_data.access_lock);
-
-	atomic_inc(&group->fanotify_data.bypass_perm);
+	fsnotify_group_stop_queueing(group);
 
+	/*
+	 * Process all permission events on access_list and notification queue
+	 * and simulate reply from userspace.
+	 */
+	spin_lock(&group->fanotify_data.access_lock);
 	list_for_each_entry_safe(event, next, &group->fanotify_data.access_list,
 				 fae.fse.list) {
 		pr_debug("%s: found group=%p event=%p\n", __func__, group,
@@ -379,12 +383,21 @@ static int fanotify_release(struct inode
 	spin_unlock(&group->fanotify_data.access_lock);
 
 	/*
-	 * Since bypass_perm is set, newly queued events will not wait for
-	 * access response. Wake up the already sleeping ones now.
-	 * synchronize_srcu() in fsnotify_destroy_group() will wait for all
-	 * processes sleeping in fanotify_handle_event() waiting for access
-	 * response and thus also for all permission events to be freed.
+	 * Destroy all non-permission events. For permission events just
+	 * dequeue them and set the response. They will be freed once the
+	 * response is consumed and fanotify_get_response() returns.
 	 */
+	mutex_lock(&group->notification_mutex);
+	while (!fsnotify_notify_queue_is_empty(group)) {
+		fsn_event = fsnotify_remove_first_event(group);
+		if (!(fsn_event->mask & FAN_ALL_PERM_EVENTS))
+			fsnotify_destroy_event(group, fsn_event);
+		else
+			FANOTIFY_PE(fsn_event)->response = FAN_ALLOW;
+	}
+	mutex_unlock(&group->notification_mutex);
+
+	/* Response for all permission events it set, wakeup waiters */
 	wake_up(&group->fanotify_data.access_waitq);
 #endif
 
@@ -755,7 +768,6 @@ SYSCALL_DEFINE2(fanotify_init, unsigned
 	spin_lock_init(&group->fanotify_data.access_lock);
 	init_waitqueue_head(&group->fanotify_data.access_waitq);
 	INIT_LIST_HEAD(&group->fanotify_data.access_list);
-	atomic_set(&group->fanotify_data.bypass_perm, 0);
 #endif
 	switch (flags & FAN_ALL_CLASS_BITS) {
 	case FAN_CLASS_NOTIF:
--- a/fs/notify/notification.c
+++ b/fs/notify/notification.c
@@ -132,21 +132,6 @@ queue:
 }
 
 /*
- * Remove @event from group's notification queue. It is the responsibility of
- * the caller to destroy the event.
- */
-void fsnotify_remove_event(struct fsnotify_group *group,
-			   struct fsnotify_event *event)
-{
-	mutex_lock(&group->notification_mutex);
-	if (!list_empty(&event->list)) {
-		list_del_init(&event->list);
-		group->q_len--;
-	}
-	mutex_unlock(&group->notification_mutex);
-}
-
-/*
  * Remove and return the first event from the notification list.  It is the
  * responsibility of the caller to destroy the obtained event
  */
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -180,7 +180,6 @@ struct fsnotify_group {
 			spinlock_t access_lock;
 			struct list_head access_list;
 			wait_queue_head_t access_waitq;
-			atomic_t bypass_perm;
 #endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */
 			int f_flags;
 			unsigned int max_marks;
@@ -318,8 +317,6 @@ extern int fsnotify_add_event(struct fsn
 			      struct fsnotify_event *event,
 			      int (*merge)(struct list_head *,
 					   struct fsnotify_event *));
-/* Remove passed event from groups notification queue */
-extern void fsnotify_remove_event(struct fsnotify_group *group, struct fsnotify_event *event);
 /* true if the group notification queue is empty */
 extern bool fsnotify_notify_queue_is_empty(struct fsnotify_group *group);
 /* return, but do not dequeue the first event on the notification queue */

  parent reply	other threads:[~2016-09-28  9:04 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20160928090645uscas1p25d8333ed85efe1cdd54dcaecedfc3b84@uscas1p2.samsung.com>
2016-09-28  9:04 ` [PATCH 4.7 00/69] 4.7.6-stable review Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 01/69] reiserfs: fix "new_insert_key may be used uninitialized ..." Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 02/69] crypto: arm64/aes-ctr - fix NULL dereference in tail processing Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 03/69] crypto: arm/aes-ctr " Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 04/69] crypto: skcipher - Fix blkcipher walk OOM crash Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 05/69] crypto: echainiv - Replace chaining with multiplication Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 06/69] ocfs2/dlm: fix race between convert and migration Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 07/69] ocfs2: fix start offset to ocfs2_zero_range_for_truncate() Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 08/69] Revert "ocfs2: bump up o2cb network protocol version" Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 09/69] autofs: use dentry flags to block walks during expire Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 10/69] Disable "maybe-uninitialized" warning globally Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 12/69] Makefile: Mute warning for __builtin_return_address(>0) for tracing only Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 13/69] xfs: prevent dropping ioend completions during buftarg wait Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 14/69] mm: fix the page_swap_info() BUG_ON check Greg Kroah-Hartman
2016-09-28  9:04   ` [PATCH 4.7 15/69] fsnotify: add a way to stop queueing events on group shutdown Greg Kroah-Hartman
2016-09-28  9:04   ` Greg Kroah-Hartman [this message]
2016-09-28  9:04   ` [PATCH 4.7 17/69] mm: memcontrol: make per-cpu charge cache IRQ-safe for socket accounting Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 18/69] cgroup: duplicate cgroup reference when cloning sockets Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 19/69] fix fault_in_multipages_...() on architectures with no-op access_ok() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 20/69] KEYS: Fix skcipher IV clobbering Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 21/69] arm64: Call numa_store_cpu_info() earlier Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 22/69] configfs: Return -EFBIG from configfs_write_bin_file Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 24/69] mtd: maps: sa1100-flash: potential NULL dereference Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 25/69] mtd: pmcmsp-flash: Allocating too much in init_msp_flash() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 26/69] mtd: spi-nor: fix wrong "fully unlocked" test Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 27/69] reset: Return -ENOTSUPP when not configured Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 28/69] rtc: ds1307: Fix relying on reset value for weekday Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 29/69] power: reset: hisi-reboot: Unmap region obtained by of_iomap Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 30/69] mac80211: reject TSPEC TIDs (TSIDs) for aggregation Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 31/69] fix memory leaks in tracing_buffers_splice_read() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 32/69] tracing: Move mutex to protect against resetting of seq data Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 33/69] mm: delete unnecessary and unsafe init_tlb_ubc() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 34/69] can: flexcan: fix resume function Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 35/69] net: can: ifi: Configure transmitter delay Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 36/69] iwlwifi: mvm: update TX queue before making a copy of the skb Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 37/69] nl80211: validate number of probe response CSA counters Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 38/69] btrfs: ensure that file descriptor used with subvol ioctls is a dir Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 39/69] x86/efi: Only map RAM into EFI page tables if in mixed-mode Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 40/69] irqchip/mips-gic: Fix local interrupts Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 41/69] i2c-eg20t: fix race between i2c init and interrupt enable Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 42/69] i2c: mux: pca954x: retry updating the mux selection on failure Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 43/69] i2c: qup: skip qup_i2c_suspend if the device is already runtime suspended Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 44/69] MIPS: Fix pre-r6 emulation FPU initialisation Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 45/69] MIPS: SMP: Fix possibility of deadlock when bringing CPUs online Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 46/69] MIPS: vDSO: Fix Malta EVA mapping to vDSO page structs Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 47/69] MIPS: Remove compact branch policy Kconfig entries Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 48/69] MIPS: Avoid a BUG warning during prctl(PR_SET_FP_MODE, ...) Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 49/69] MIPS: Add a missing ".set pop" in an early commit Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 50/69] MIPS: paravirt: Fix undefined reference to smp_bootstrap Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 51/69] x86/mm/pat: Prevent hang during boot when mapping pages Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 52/69] libceph: add an ONSTACK initializer for oids Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 53/69] ceph: fix symbol versioning for ceph_monc_do_statfs Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 54/69] ceph: Correctly return NXIO errors from ceph_llseek Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 55/69] libceph: fix return value check in alloc_msg_with_page_vector() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 56/69] PM / hibernate: Restore processor state before using per-CPU variables Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 57/69] PM / hibernate: Fix rtree_next_node() to avoid walking off list ends Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 58/69] power_supply: tps65217-charger: fix missing platform_set_drvdata() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 59/69] power: supply: max17042_battery: fix model download bug Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 60/69] ixgbe: Force VLNCTRL.VFE to be set in all VMDq paths Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 61/69] ixgbe: Re-enable ability to toggle VLAN filtering Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 62/69] igb: fix adjusting PTP timestamps for Tx/Rx latency Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 63/69] soc/tegra: pmc: Dont probe PMC if early initialisation fails Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 64/69] qxl: check for kmap failures Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 65/69] hostfs: Freeing an ERR_PTR in hostfs_fill_sb_common() Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 66/69] kasan: avoid overflowing quarantine size on low memory systems Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 67/69] mm/kasan: dont reduce quarantine in atomic contexts Greg Kroah-Hartman
2016-09-28  9:05   ` [PATCH 4.7 68/69] iw_cxgb4: stop MPA_REPLY timer when disconnecting Greg Kroah-Hartman
2016-09-28 16:46   ` [PATCH 4.7 00/69] 4.7.6-stable review Shuah Khan
2016-09-29  9:00     ` Greg Kroah-Hartman
2016-09-28 22:44   ` Guenter Roeck
2016-09-29  9:00     ` Greg Kroah-Hartman
     [not found]   ` <57ec0654.4756c20a.c7ddf.48d1@mx.google.com>
     [not found]     ` <7ha8erru8h.fsf@baylibre.com>
2016-09-29  8:33       ` Greg Kroah-Hartman
2016-09-29 14:46         ` Kevin Hilman
2016-09-29 15:26           ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160928090445.779296493@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=jack@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).