linux-kernel - epoll: possible bug from wakeup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130307112639.GA25130@dcvr.yhbt.net>
Date:	Thu, 7 Mar 2013 11:26:39 +0000
From:	Eric Wong <normalperson@...t.net>
To:	Arve Hjønnevåg <arve@...roid.com>
Cc:	NeilBrown <neilb@...e.de>, "Rafael J. Wysocki" <rjw@...k.pl>,
	linux-kernel@...r.kernel.org,
	Davide Libenzi <davidel@...ilserver.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: epoll: possible bug from wakeup_source activation

Hi Arve, looking at commit 4d7e30d98939a0340022ccd49325a3d70f7e0238
(epoll: Add a flag, EPOLLWAKEUP, to prevent suspend ...)

I think the reason for using ep->ws instead of epi->ws in the unlikely
ovflist case applies to the likely rdllist case, too.  Since epi->ws is
only protected by ep->mtx, it can also be deactivated while inside
ep_poll_callback.

So something like the following patch might be necessary
(shown here with extra context):

--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -968,39 +968,45 @@ static int ep_poll_callback(wait_queue_t *wait, unsigned mode, int sync, void *k
 	if (unlikely(ep->ovflist != EP_UNACTIVE_PTR)) {
 		if (epi->next == EP_UNACTIVE_PTR) {
 			epi->next = ep->ovflist;
 			ep->ovflist = epi;
 			if (epi->ws) {
 				/*
 				 * Activate ep->ws since epi->ws may get
 				 * deactivated at any time.
 				 */
 				__pm_stay_awake(ep->ws);
 			}
 
 		}
 		goto out_unlock;
 	}
 
 	/* If this file is already in the ready list we exit soon */
 	if (!ep_is_linked(&epi->rdllink)) {
 		list_add_tail(&epi->rdllink, &ep->rdllist);
-		__pm_stay_awake(epi->ws);
+		if (epi->ws) {
+			/*
+			 * Activate ep->ws since epi->ws may get
+			 * deactivated at any time.
+			 */
+			__pm_stay_awake(ep->ws);
+		}
 	}
-----------------------------------------------------------------------
However, I think my proposed patch will also cause breakage with the
way epi->ws is handled in ep_send_events_proc:

		/*
		 * Activate ep->ws before deactivating epi->ws to prevent
		 * triggering auto-suspend here (in case we reactive epi->ws
		 * below).
		 *
		 * This could be rearranged to delay the deactivation of epi->ws
		 * instead, but then epi->ws would temporarily be out of sync
		 * with ep_is_linked().
		 */
		if (epi->ws && epi->ws->active)
			__pm_stay_awake(ep->ws);
		__pm_relax(epi->ws);
		list_del_init(&epi->rdllink);

I'm not sure, but maybe only using ep->ws is the easiest way to go.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/