From patchwork Fri Nov 17 14:11:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adam Wallis X-Patchwork-Id: 10062819 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1553F60352 for ; Fri, 17 Nov 2017 14:12:16 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 06ADB2ACDF for ; Fri, 17 Nov 2017 14:12:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id EE7982ACE1; Fri, 17 Nov 2017 14:12:15 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8D3772ACDF for ; Fri, 17 Nov 2017 14:12:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Message-Id:Date: Subject:To:From:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To: References:List-Owner; bh=vfMyuZHRu2j7Ok9veGrBqF4OywwoJTavd9WxAq129lE=; b=nqM AF537ZoDPrhEhS4+zokC941he+mP+3DNgLryLbgN4QHDosUCj4VF+MDBQqjOZpkebpjUQppwek8jw Ix5yRvd0elUsdDPmZcpzv61FYaP3MtyuT/Eh6IXZykgRX697GIhJ7LSCeuntn/O8NpsOalgNouUqU e7cTObXwANOVVfrH22tTMQffVCgGRGc7ld2yQ5e7t/39AH8bkgnFnXbPvcLM2o0XmnHd+IFEHmfcN aXNDmT8vDi/ZVp8Bz+yQI5fEfPJ5TOJDGaHaDl3VMy2HaabFH4lzoIwPUCC/bRKJdTxC8+KrGfxaa 4QT7ijqF+SRYA6Xrb2yYj8zEcbv+Nnw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1eFhNM-0003tc-Ja; Fri, 17 Nov 2017 14:12:12 +0000 Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1eFhNF-0003n7-Ed for linux-arm-kernel@lists.infradead.org; Fri, 17 Nov 2017 14:12:11 +0000 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 61053607A1; Fri, 17 Nov 2017 14:11:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1510927904; bh=+ZFAwX+1Um9etsV+Vn+Zd1DRuHbgtXkKEekUrvq6fuU=; h=From:To:Cc:Subject:Date:From; b=ltR68Fy9AOjvr96JzsTJdgeMaB2+JpYNeDzVgdj2Mm7t1edSCYPNWE6RYTv8KhG0z qBOMfu/6wwAePU6wFCkzIHoWV7uAeAfYLdXFW8nNmKYYNi3chUxcwipeYf5u8CE1qw 9D5avNbyrgscVsaLWu5scUsA8la1ZIkpVni4pRZI= Received: from chromaggus.qualcomm.com (global_nat1_iad_fw.qualcomm.com [129.46.232.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: awallis@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id 8A018601CF; Fri, 17 Nov 2017 14:11:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=codeaurora.org; s=default; t=1510927903; bh=+ZFAwX+1Um9etsV+Vn+Zd1DRuHbgtXkKEekUrvq6fuU=; h=From:To:Cc:Subject:Date:From; b=H+9jpMyU1Fq2Gs+Q2ZB7pJAjC1SBZDzYlOhYwGTIUdYwWi4cLjvWFhg9cza2kERYU pF+pX6+7JVYOeVgHlLxJf9X5D2R1dvRAXmP3FTx6126iSED/0lrTvxyLyPJlchPu/s fS92qzkYEWBh6i4rdD3q1zK/rwDSzscDwuJKSO/Q= DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 8A018601CF Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=awallis@codeaurora.org From: Adam Wallis To: linux-arm-kernel@lists.infradead.org, Dan Williams , Vinod Koul , Sinan Kaya , dmaengine@vger.kernel.org, shunyong.yang@hxt-semitech.com Subject: [PATCH v3][for 4.15] dmaengine: dmatest: move callback wait queue to thread context Date: Fri, 17 Nov 2017 09:11:34 -0500 Message-Id: <1510927894-15297-1-git-send-email-awallis@codeaurora.org> X-Mailer: git-send-email 1.9.1 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171117_061205_539232_4DBF3570 X-CRM114-Status: GOOD ( 19.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: timur@codeaurora.org MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Commit adfa543e7314 ("dmatest: don't use set_freezable_with_signal()") introduced a bug (that is in fact documented by the patch commit text) that leaves behind a dangling pointer. Since the done_wait structure is allocated on the stack, future invocations to the DMATEST can produce undesirable results (e.g., corrupted spinlocks). Commit a9df21e34b42 ("dmaengine: dmatest: warn user when dma test times out") attempted to WARN the user that the stack was likely corrupted but did not fix the actual issue. This patch fixes the issue by pushing the wait queue and callback structs into the the thread structure. If a failure occurs due to time, dmaengine_terminate_all will force the callback to safely call wake_up_all() without possibility of using a freed pointer. Cc: stable@vger.kernel.org # 4.13.x: a9df21e: dmatest: Warn User Cc: stable@vger.kernel.org # 4.13.x Cc: stable@vger.kernel.org # 4.14.x Bug: https://bugzilla.kernel.org/show_bug.cgi?id=197605 Fixes: adfa543e7314 ("dmatest: don't use set_freezable_with_signal()") Reviewed-by: Sinan Kaya Suggested-by: Shunyong Yang Signed-off-by: Adam Wallis --- changes from v2: Added "Fixes" tag changes from v1: Added pre-req patches for stable drivers/dma/dmatest.c | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c index 47edc7f..2573b6c 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -155,6 +155,12 @@ struct dmatest_params { #define PATTERN_COUNT_MASK 0x1f #define PATTERN_MEMSET_IDX 0x01 +/* poor man's completion - we want to use wait_event_freezable() on it */ +struct dmatest_done { + bool done; + wait_queue_head_t *wait; +}; + struct dmatest_thread { struct list_head node; struct dmatest_info *info; @@ -165,6 +171,8 @@ struct dmatest_thread { u8 **dsts; u8 **udsts; enum dma_transaction_type type; + wait_queue_head_t done_wait; + struct dmatest_done test_done; bool done; }; @@ -342,11 +350,6 @@ static unsigned int dmatest_verify(u8 **bufs, unsigned int start, return error_count; } -/* poor man's completion - we want to use wait_event_freezable() on it */ -struct dmatest_done { - bool done; - wait_queue_head_t *wait; -}; static void dmatest_callback(void *arg) { @@ -424,9 +427,8 @@ static unsigned long long dmatest_KBs(s64 runtime, unsigned long long len) */ static int dmatest_func(void *data) { - DECLARE_WAIT_QUEUE_HEAD_ONSTACK(done_wait); struct dmatest_thread *thread = data; - struct dmatest_done done = { .wait = &done_wait }; + struct dmatest_done *done = &thread->test_done; struct dmatest_info *info; struct dmatest_params *params; struct dma_chan *chan; @@ -673,9 +675,9 @@ static int dmatest_func(void *data) continue; } - done.done = false; + done->done = false; tx->callback = dmatest_callback; - tx->callback_param = &done; + tx->callback_param = done; cookie = tx->tx_submit(tx); if (dma_submit_error(cookie)) { @@ -688,21 +690,12 @@ static int dmatest_func(void *data) } dma_async_issue_pending(chan); - wait_event_freezable_timeout(done_wait, done.done, + wait_event_freezable_timeout(thread->done_wait, done->done, msecs_to_jiffies(params->timeout)); status = dma_async_is_tx_complete(chan, cookie, NULL, NULL); - if (!done.done) { - /* - * We're leaving the timed out dma operation with - * dangling pointer to done_wait. To make this - * correct, we'll need to allocate wait_done for - * each test iteration and perform "who's gonna - * free it this time?" dancing. For now, just - * leave it dangling. - */ - WARN(1, "dmatest: Kernel stack may be corrupted!!\n"); + if (!done->done) { dmaengine_unmap_put(um); result("test timed out", total_tests, src_off, dst_off, len, 0); @@ -789,7 +782,7 @@ static int dmatest_func(void *data) dmatest_KBs(runtime, total_len), ret); /* terminate all transfers on specified channels */ - if (ret) + if (ret || failed_tests) dmaengine_terminate_all(chan); thread->done = true; @@ -849,6 +842,8 @@ static int dmatest_add_threads(struct dmatest_info *info, thread->info = info; thread->chan = dtc->chan; thread->type = type; + thread->test_done.wait = &thread->done_wait; + init_waitqueue_head(&thread->done_wait); smp_wmb(); thread->task = kthread_create(dmatest_func, thread, "%s-%s%u", dma_chan_name(chan), op, i);