From nobody Fri Feb 25 19:58:58 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id C669019E12CF; Fri, 25 Feb 2022 19:58:58 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4K50w25DHJz3PDB; Fri, 25 Feb 2022 19:58:58 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1645819138; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=piS954veQ4kFCbpuKuQx6NWpZesY6mXSeDpjhTqW90E=; b=G/bVNzNf6VQPfF8FRalC7c7aG6jZB+u3o0ijgO4zpGbPPdK+QzhvMq0TZWAXsz7kiiL1HW Fc7/z737MIMWqJG2MhV27tmkbA+lNAoyJ8eKeYS4Dx6aOUy+QGvFZ1C0FE+02PYQuUhiHw 04ctfxFJ+vODTeNZo8bZI08y90Im5r8N1Ybb9erOqubvL3isSfpxZw+dWFVHdtkt07KqHz vcig7/xq1Qq6PLM2jcZzGE0GJ24OrRDqHKRjRqKSx8DzQvUtp6EKoygNJy0P2zB4Uwy96G uV+yehk6kRhBM6BY4O58lXrthzg2PMdL/yfh34v0Nmi/BxE8apKfsXnbqN4JKg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 8F22825B58; Fri, 25 Feb 2022 19:58:58 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 21PJwwpU028070; Fri, 25 Feb 2022 19:58:58 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 21PJwwYH028069; Fri, 25 Feb 2022 19:58:58 GMT (envelope-from git) Date: Fri, 25 Feb 2022 19:58:58 GMT Message-Id: <202202251958.21PJwwYH028069@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Ryan Stone Subject: git: 1708d1b88972 - stable/13 - MFC b58cf1cb35ab: List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rstone X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 1708d1b88972fe0f952926728cc861e1ddd08e38 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1645819138; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=piS954veQ4kFCbpuKuQx6NWpZesY6mXSeDpjhTqW90E=; b=tL2vEV+Nuqyj3eb4ke8vwIlRX9i7J52h6h8A5+eXbaj7TfU2NLudlIfVDUn71icMSdUoxe wrppgSAAxtY52ChC+Sw+O6cPND03oP4gCYUaRuZkDyQu+X4gloHYSeWXVLY5x3dDVIZPyU Cu6aSxK6KDm6PlOw0lRQHWGF0iDzZh9W6td6RyYaNb7feubT+0nvQaWdhCwJzhBKZEw015 Ixm9jciawnqhqY4BVZCfhAzx5Dtqa9Mcw2zmQ2pOZqiIlCJ4SW3ulz8GzJgHY4h/eshDt5 6S8ENmZ8Xh89yzba97EGnVm68yg/PVic5uuZagEhlKfIeRnjTY9oKMUzhC/z2g== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1645819138; a=rsa-sha256; cv=none; b=d0DxGRGcjWIkrKEpiCcp+TYVmoRXzxuZzxLH2PwLvw88fWBBZHUaKS/H+0KElDH7/r30I4 Ldiq+vg8Mri5tEiGHwKed7eXFOX4duTYsjVtekxDt6eCWN8md+vILYp+DH0R0xktmlwsYH GVGpV3HM8CiZ9JvcqDyLjS9nUyO0IDj9T4gD3PUMd2W8zsIfugigGN/TB+FPD2xbAAKJ6F 9p2kz4AbDpjBJm20oegU3vr4VkYsIOFlhWsSU3K+ii/UNqTTtMr4lj9mFHSC88MOoosPZi Q9E9I8K21i+RxbHP4s+VvsoHTy2ea6fQQDtxTUF/4Ovu+EHxgFZ+sLcL8zIb7g== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by rstone: URL: https://cgit.FreeBSD.org/src/commit/?id=1708d1b88972fe0f952926728cc861e1ddd08e38 commit 1708d1b88972fe0f952926728cc861e1ddd08e38 Author: Ryan Stone AuthorDate: 2021-01-07 17:25:56 +0000 Commit: Ryan Stone CommitDate: 2022-02-25 19:57:51 +0000 MFC b58cf1cb35ab: Fix race condition in linuxkpi workqueue Consider the following scenario: 1. A delayed_work struct in the WORK_ST_TIMER state. 2. Thread A calls mod_delayed_work() 3. Thread B (a callout thread) simultaneously calls linux_delayed_work_timer_fn() The following sequence of events is possible: A: Call linux_cancel_delayed_work() A: Change state from TIMER TO CANCEL B: Change state from CANCEL to TASK B: taskqueue_enqueue() the task A: taskqueue_cancel() the task A: Call linux_queue_delayed_work_on(). This is a no-op because the state is WORK_ST_TASK. As a result, the delayed_work struct will never be invoked. This is causing address resolution in ib_addr.c to stop permanently, as it never tries to reschedule a task that it thinks is already scheduled. Fix this by introducing locking into the cancel path (which corresponds with the lock held while the callout runs). This will prevent the callout from changing the state of the task until the cancel is complete, preventing the race. Differential Revision: https://reviews.freebsd.org/D28420 Reviewed by: hselasky MFC after: 2 months (cherry picked from commit b58cf1cb35abc095d7fb9773630794b38bbc6b1d) --- sys/compat/linuxkpi/common/src/linux_work.c | 49 ++++++++++++++++------------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_work.c b/sys/compat/linuxkpi/common/src/linux_work.c index 45025378baa9..f9cf62928760 100644 --- a/sys/compat/linuxkpi/common/src/linux_work.c +++ b/sys/compat/linuxkpi/common/src/linux_work.c @@ -435,13 +435,17 @@ linux_cancel_delayed_work(struct delayed_work *dwork) [WORK_ST_CANCEL] = WORK_ST_CANCEL, /* NOP */ }; struct taskqueue *tq; + bool cancelled; + mtx_lock(&dwork->timer.mtx); switch (linux_update_state(&dwork->work.state, states)) { case WORK_ST_TIMER: case WORK_ST_CANCEL: - if (linux_cancel_timer(dwork, 0)) { + cancelled = (callout_stop(&dwork->timer.callout) == 1); + if (cancelled) { atomic_cmpxchg(&dwork->work.state, WORK_ST_CANCEL, WORK_ST_IDLE); + mtx_unlock(&dwork->timer.mtx); return (true); } /* FALLTHROUGH */ @@ -450,10 +454,12 @@ linux_cancel_delayed_work(struct delayed_work *dwork) if (taskqueue_cancel(tq, &dwork->work.work_task, NULL) == 0) { atomic_cmpxchg(&dwork->work.state, WORK_ST_CANCEL, WORK_ST_IDLE); + mtx_unlock(&dwork->timer.mtx); return (true); } /* FALLTHROUGH */ default: + mtx_unlock(&dwork->timer.mtx); return (false); } } @@ -475,37 +481,36 @@ linux_cancel_delayed_work_sync(struct delayed_work *dwork) }; struct taskqueue *tq; bool retval = false; + int ret, state; + bool cancelled; WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "linux_cancel_delayed_work_sync() might sleep"); -retry: - switch (linux_update_state(&dwork->work.state, states)) { + mtx_lock(&dwork->timer.mtx); + + state = linux_update_state(&dwork->work.state, states); + switch (state) { case WORK_ST_IDLE: + mtx_unlock(&dwork->timer.mtx); return (retval); - case WORK_ST_EXEC: - tq = dwork->work.work_queue->taskqueue; - if (taskqueue_cancel(tq, &dwork->work.work_task, NULL) != 0) - taskqueue_drain(tq, &dwork->work.work_task); - goto retry; /* work may have restarted itself */ case WORK_ST_TIMER: case WORK_ST_CANCEL: - if (linux_cancel_timer(dwork, 1)) { - /* - * Make sure taskqueue is also drained before - * returning: - */ - tq = dwork->work.work_queue->taskqueue; - taskqueue_drain(tq, &dwork->work.work_task); - retval = true; - goto retry; - } - /* FALLTHROUGH */ + cancelled = (callout_stop(&dwork->timer.callout) == 1); + + tq = dwork->work.work_queue->taskqueue; + ret = taskqueue_cancel(tq, &dwork->work.work_task, NULL); + mtx_unlock(&dwork->timer.mtx); + + callout_drain(&dwork->timer.callout); + taskqueue_drain(tq, &dwork->work.work_task); + return (cancelled || (ret != 0)); default: tq = dwork->work.work_queue->taskqueue; - if (taskqueue_cancel(tq, &dwork->work.work_task, NULL) != 0) + ret = taskqueue_cancel(tq, &dwork->work.work_task, NULL); + mtx_unlock(&dwork->timer.mtx); + if (ret != 0) taskqueue_drain(tq, &dwork->work.work_task); - retval = true; - goto retry; + return (ret != 0); } }