mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org
Cc: stable-review@kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	Davide Libenzi <davidel@xmailserver.org>,
	Nelson Elhage <nelhage@ksplice.com>
Subject: [61/68] epoll: prevent creating circular epoll structures
Date: Mon, 28 Feb 2011 08:23:22 -0800	[thread overview]
Message-ID: <20110228162411.831717100@clark.kroah.org> (raw)
In-Reply-To: <20110228163502.GA27221@kroah.com>

2.6.32-longterm review patch.  If anyone has any objections, please let us know.

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

From: Davide Libenzi <davidel@xmailserver.org>

commit 22bacca48a1755f79b7e0f192ddb9fbb7fc6e64e upstream.

In several places, an epoll fd can call another file's ->f_op->poll()
method with ep->mtx held.  This is in general unsafe, because that other
file could itself be an epoll fd that contains the original epoll fd.

The code defends against this possibility in its own ->poll() method using
ep_call_nested, but there are several other unsafe calls to ->poll
elsewhere that can be made to deadlock.  For example, the following simple
program causes the call in ep_insert recursively call the original fd's
->poll, leading to deadlock:

 #include <unistd.h>
 #include <sys/epoll.h>

 int main(void) {
     int e1, e2, p[2];
     struct epoll_event evt = {
         .events = EPOLLIN
     };

     e1 = epoll_create(1);
     e2 = epoll_create(2);
     pipe(p);

     epoll_ctl(e2, EPOLL_CTL_ADD, e1, &evt);
     epoll_ctl(e1, EPOLL_CTL_ADD, p[0], &evt);
     write(p[1], p, sizeof p);
     epoll_ctl(e1, EPOLL_CTL_ADD, e2, &evt);

     return 0;
 }

On insertion, check whether the inserted file is itself a struct epoll,
and if so, do a recursive walk to detect whether inserting this file would
create a loop of epoll structures, which could lead to deadlock.

[nelhage@ksplice.com: Use epmutex to serialize concurrent inserts]
Signed-off-by: Davide Libenzi <davidel@xmailserver.org>
Signed-off-by: Nelson Elhage <nelhage@ksplice.com>
Reported-by: Nelson Elhage <nelhage@ksplice.com>
Tested-by: Nelson Elhage <nelhage@ksplice.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@suse.de>

---
 fs/eventpoll.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -63,6 +63,13 @@
  * cleanup path and it is also acquired by eventpoll_release_file()
  * if a file has been pushed inside an epoll set and it is then
  * close()d without a previous call toepoll_ctl(EPOLL_CTL_DEL).
+ * It is also acquired when inserting an epoll fd onto another epoll
+ * fd. We do this so that we walk the epoll tree and ensure that this
+ * insertion does not create a cycle of epoll file descriptors, which
+ * could lead to deadlock. We need a global mutex to prevent two
+ * simultaneous inserts (A into B and B into A) from racing and
+ * constructing a cycle without either insert observing that it is
+ * going to.
  * It is possible to drop the "ep->mtx" and to use the global
  * mutex "epmutex" (together with "ep->lock") to have it working,
  * but having "ep->mtx" will make the interface more scalable.
@@ -227,6 +234,9 @@ static int max_user_watches __read_mostl
  */
 static DEFINE_MUTEX(epmutex);
 
+/* Used to check for epoll file descriptor inclusion loops */
+static struct nested_calls poll_loop_ncalls;
+
 /* Used for safe wake up implementation */
 static struct nested_calls poll_safewake_ncalls;
 
@@ -1182,6 +1192,62 @@ retry:
 	return res;
 }
 
+/**
+ * ep_loop_check_proc - Callback function to be passed to the @ep_call_nested()
+ *                      API, to verify that adding an epoll file inside another
+ *                      epoll structure, does not violate the constraints, in
+ *                      terms of closed loops, or too deep chains (which can
+ *                      result in excessive stack usage).
+ *
+ * @priv: Pointer to the epoll file to be currently checked.
+ * @cookie: Original cookie for this call. This is the top-of-the-chain epoll
+ *          data structure pointer.
+ * @call_nests: Current dept of the @ep_call_nested() call stack.
+ *
+ * Returns: Returns zero if adding the epoll @file inside current epoll
+ *          structure @ep does not violate the constraints, or -1 otherwise.
+ */
+static int ep_loop_check_proc(void *priv, void *cookie, int call_nests)
+{
+	int error = 0;
+	struct file *file = priv;
+	struct eventpoll *ep = file->private_data;
+	struct rb_node *rbp;
+	struct epitem *epi;
+
+	mutex_lock(&ep->mtx);
+	for (rbp = rb_first(&ep->rbr); rbp; rbp = rb_next(rbp)) {
+		epi = rb_entry(rbp, struct epitem, rbn);
+		if (unlikely(is_file_epoll(epi->ffd.file))) {
+			error = ep_call_nested(&poll_loop_ncalls, EP_MAX_NESTS,
+					       ep_loop_check_proc, epi->ffd.file,
+					       epi->ffd.file->private_data, current);
+			if (error != 0)
+				break;
+		}
+	}
+	mutex_unlock(&ep->mtx);
+
+	return error;
+}
+
+/**
+ * ep_loop_check - Performs a check to verify that adding an epoll file (@file)
+ *                 another epoll file (represented by @ep) does not create
+ *                 closed loops or too deep chains.
+ *
+ * @ep: Pointer to the epoll private data structure.
+ * @file: Pointer to the epoll file to be checked.
+ *
+ * Returns: Returns zero if adding the epoll @file inside current epoll
+ *          structure @ep does not violate the constraints, or -1 otherwise.
+ */
+static int ep_loop_check(struct eventpoll *ep, struct file *file)
+{
+	return ep_call_nested(&poll_loop_ncalls, EP_MAX_NESTS,
+			      ep_loop_check_proc, file, ep, current);
+}
+
 /*
  * Open an eventpoll file descriptor.
  */
@@ -1230,6 +1296,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, in
 		struct epoll_event __user *, event)
 {
 	int error;
+	int did_lock_epmutex = 0;
 	struct file *file, *tfile;
 	struct eventpoll *ep;
 	struct epitem *epi;
@@ -1271,6 +1338,25 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, in
 	 */
 	ep = file->private_data;
 
+	/*
+	 * When we insert an epoll file descriptor, inside another epoll file
+	 * descriptor, there is the change of creating closed loops, which are
+	 * better be handled here, than in more critical paths.
+	 *
+	 * We hold epmutex across the loop check and the insert in this case, in
+	 * order to prevent two separate inserts from racing and each doing the
+	 * insert "at the same time" such that ep_loop_check passes on both
+	 * before either one does the insert, thereby creating a cycle.
+	 */
+	if (unlikely(is_file_epoll(tfile) && op == EPOLL_CTL_ADD)) {
+		mutex_lock(&epmutex);
+		did_lock_epmutex = 1;
+		error = -ELOOP;
+		if (ep_loop_check(ep, tfile) != 0)
+			goto error_tgt_fput;
+	}
+
+
 	mutex_lock(&ep->mtx);
 
 	/*
@@ -1306,6 +1392,9 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, in
 	mutex_unlock(&ep->mtx);
 
 error_tgt_fput:
+	if (unlikely(did_lock_epmutex))
+		mutex_unlock(&epmutex);
+
 	fput(tfile);
 error_fput:
 	fput(file);
@@ -1424,6 +1513,12 @@ static int __init eventpoll_init(void)
 	max_user_watches = (((si.totalram - si.totalhigh) / 25) << PAGE_SHIFT) /
 		EP_ITEM_COST;
 
+	/*
+	 * Initialize the structure used to perform epoll file descriptor
+	 * inclusion loops checks.
+	 */
+	ep_nested_calls_init(&poll_loop_ncalls);
+
 	/* Initialize the structure used to perform safe poll wait head wake ups */
 	ep_nested_calls_init(&poll_safewake_ncalls);
 



  parent reply	other threads:[~2011-02-28 16:38 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-28 16:35 [00/68] 2.6.32.30-longterm review Greg KH
2011-02-28 16:22 ` [01/68] NFSD: memory corruption due to writing beyond the stat array Greg KH
2011-02-28 16:22 ` [02/68] [SCSI] mptfusion: mptctl_release is required in mptctl.c Greg KH
2011-02-28 16:22 ` [03/68] [SCSI] mptfusion: Fix Incorrect return value in mptscsih_dev_reset Greg KH
2011-02-28 16:22 ` [04/68] sctp: Fix out-of-bounds reading in sctp_asoc_get_hmac() Greg KH
2011-02-28 16:22 ` [05/68] ocfs2_connection_find() returns pointer to bad structure Greg KH
2011-02-28 16:22 ` [06/68] Fix pktcdvd ioctl dev_minor range check Greg KH
2011-02-28 16:22 ` [07/68] filter: make sure filters dont read uninitialized memory Greg KH
2011-02-28 16:22 ` [08/68] x25: decrement netdev reference counts on unload Greg KH
2011-02-28 16:22 ` [09/68] x86, hpet: Disable per-cpu hpet timer if ARAT is supported Greg KH
2011-02-28 16:22 ` [10/68] OHCI: work around for nVidia shutdown problem Greg KH
2011-02-28 16:22 ` [11/68] x86/pvclock: Zero last_value on resume Greg KH
2011-02-28 16:22 ` [12/68] [media] [v3,media] av7110: check for negative array offset Greg KH
2011-02-28 16:22 ` [13/68] CRED: Fix get_task_cred() and task_state() to not resurrect dead credentials Greg KH
2011-02-28 16:22 ` [14/68] bonding/vlan: Avoid mangled NAs on slaves without VLAN tag insertion Greg KH
2011-02-28 16:22 ` [15/68] CRED: Fix kernel panic upon security_file_alloc() failure Greg KH
2011-02-28 16:22 ` [16/68] CRED: Fix BUG() upon security_cred_alloc_blank() failure Greg KH
2011-02-28 16:22 ` [17/68] CRED: Fix memory and refcount leaks upon security_prepare_creds() failure Greg KH
2011-02-28 16:22 ` [18/68] sendfile(): check f_op.splice_write() rather than f_op.sendpage() Greg KH
2011-02-28 16:22 ` [19/68] NFS: fix the return value of nfs_file_fsync() Greg KH
2011-02-28 16:22 ` [20/68] isdn: hisax: Replace the bogus access to irq stats Greg KH
2011-02-28 16:22 ` [21/68] ixgbe: add support for 82599 based Express Module X520-P2 Greg KH
2011-02-28 16:22 ` [22/68] ixgbe: prevent speculative processing of descriptors before ready Greg KH
2011-03-01  2:14   ` [Stable-review] " Ben Hutchings
2011-03-01 20:46     ` Greg KH
2011-03-01 21:56       ` Jeff Kirsher
2011-02-28 16:22 ` [23/68] [SCSI] scsi_dh_alua: add netapp to dev list Greg KH
2011-02-28 16:22 ` [24/68] [SCSI] scsi_dh_alua: Add IBM Power Virtual SCSI ALUA device " Greg KH
2011-02-28 16:22 ` [25/68] dm raid1: fail writes if errors are not handled and log fails Greg KH
2011-02-28 16:22 ` [26/68] GFS2: Fix bmap allocation corner-case bug Greg KH
2011-02-28 16:22 ` [27/68] dm raid1: fix null pointer dereference in suspend Greg KH
2011-02-28 16:22 ` [28/68] sunrpc/cache: fix module refcnt leak in a failure path Greg KH
2011-02-28 16:22 ` [29/68] be2net: Maintain tx and rx counters in driver Greg KH
2011-02-28 16:22 ` [30/68] tcp: Increase TCP_MAXSEG socket option minimum Greg KH
2011-02-28 16:22 ` [31/68] tcp: Make TCP_MAXSEG minimum more correct Greg KH
2011-02-28 16:22 ` [32/68] nfsd: correctly handle return value from nfsd_map_name_to_* Greg KH
2011-02-28 16:22 ` [33/68] xfs: always use iget in bulkstat Greg KH
2011-02-28 16:22 ` [34/68] xfs: validate untrusted inode numbers during lookup Greg KH
2011-02-28 16:22 ` [35/68] xfs: rename XFS_IGET_BULKSTAT to XFS_IGET_UNTRUSTED Greg KH
2011-02-28 16:22 ` [36/68] xfs: remove block number from inode lookup code Greg KH
2011-02-28 16:22 ` [37/68] xfs: fix untrusted inode number lookup Greg KH
2011-02-28 16:22 ` [38/68] s390: remove task_show_regs Greg KH
2011-02-28 16:23 ` [39/68] PM / Hibernate: Return error code when alloc_image_page() fails Greg KH
2011-02-28 16:23 ` [40/68] fs/partitions: Validate map_count in Mac partition tables Greg KH
2011-02-28 16:23 ` [41/68] ALSA: HDA: Add position_fix quirk for an Asus device Greg KH
2011-02-28 16:23 ` [42/68] ALSA: caiaq - Fix possible string-buffer overflow Greg KH
2011-02-28 16:23 ` [43/68] [media] radio-aimslab.c needs #include <linux/delay.h> Greg KH
2011-02-28 16:23 ` [44/68] ARM: Ensure predictable endian state on signal handler entry Greg KH
2011-02-28 16:23 ` [45/68] acer-wmi: Fix capitalisation of GUID Greg KH
2011-02-28 16:23 ` [46/68] eCryptfs: Copy up lower inode attrs in getattr Greg KH
2011-02-28 16:23 ` [47/68] platform: x86: acer-wmi: world-writable sysfs threeg file Greg KH
2011-02-28 16:23 ` [48/68] platform: x86: asus_acpi: world-writable procfs files Greg KH
2011-02-28 16:23 ` [49/68] platform: x86: tc1100-wmi: world-writable sysfs wireless and jogdial files Greg KH
2011-02-28 16:23 ` [50/68] genirq: Disable the SHIRQ_DEBUG call in request_threaded_irq for now Greg KH
2011-02-28 16:23 ` [51/68] usb: musb: omap2430: fix kernel panic on reboot Greg KH
2011-02-28 16:23 ` [52/68] USB: add quirks entry for Keytouch QWERTY Panel Greg KH
2011-02-28 16:23 ` [53/68] USB: Add Samsung SGH-I500/Android modem ID switch to visor driver Greg KH
2011-02-28 16:23 ` [54/68] USB: Add quirk for Samsung Android phone modem Greg KH
2011-02-28 16:23 ` [55/68] p54pci: update receive dma buffers before and after processing Greg KH
2011-02-28 16:23 ` [56/68] sierra: add new ID for Airprime/Sierra USB IP modem Greg KH
2011-02-28 16:23 ` [57/68] staging: usbip: vhci: update reference count for usb_device Greg KH
2011-02-28 16:23 ` [58/68] staging: usbip: vhci: give back URBs from in-flight unlink requests Greg KH
2011-02-28 16:23 ` [59/68] staging: usbip: vhci: refuse to enqueue for dead connections Greg KH
2011-02-28 16:23 ` [60/68] staging: usbip: vhci: use urb->dev->portnum to find port Greg KH
2011-02-28 16:23 ` Greg KH [this message]
2011-02-28 16:23 ` [62/68] ldm: corrupted partition table can cause kernel oops Greg KH
2011-02-28 16:23 ` [63/68] md: correctly handle probe of an mdp device Greg KH
2011-02-28 16:23 ` [64/68] x86 quirk: Fix polarity for IRQ0 pin2 override on SB800 systems Greg KH
2011-02-28 16:23 ` [65/68] xhci: Avoid BUG() in interrupt context Greg KH
2011-02-28 16:23 ` [66/68] xhci: Clarify some expressions in the TRB math Greg KH
2011-02-28 16:23 ` [67/68] xhci: Fix errors in the running total calculations " Greg KH
2011-02-28 16:23 ` [68/68] xhci: Fix an error in count_sg_trbs_needed() Greg KH

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=20110228162411.831717100@clark.kroah.org \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=davidel@xmailserver.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nelhage@ksplice.com \
    --cc=stable-review@kernel.org \
    --cc=stable@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

all inboxes | Powered by JetHome®