Received: by 10.223.185.116 with SMTP id b49csp6357308wrg; Wed, 28 Feb 2018 08:06:47 -0800 (PST) X-Google-Smtp-Source: AH8x224CzpcKFTmFpOsdsKY9Kg4oanD7xwD8WEE8+MOfRoGR//8+gaPoLte8WvAhGrpVfLuJMYTm X-Received: by 10.101.96.142 with SMTP id t14mr14449726pgu.58.1519834006849; Wed, 28 Feb 2018 08:06:46 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1519834006; cv=none; d=google.com; s=arc-20160816; b=Rf6iPL4mbt4UM//RHF9q3k9NLQvCQgBOz5tWAX5bvBHVNzE8SDn5c+gmc3psM7Z2wb AupvyiNASWllIVbexHGkvNrGjzd0pbz/uLdZGs2DvMO244kCYqN+3g0abH1s0mJvf5LR obO4qlAs2CtoVTDNJCu6Pyho/pGEdh6vo/o+1CFcimLCAeYcsKiGQDpw/Xaa/5JYGkU1 Ama4kZtBoXnTP2fx/EfAXm/cW/XXEJP9rRkbfI4boREWz+BNZb3Q3zUPsvrIr+1F4Wol Synkr3pIqGvOPz8wF4Kcf6O9BZZiV3un8mZQ/E38pta/zU2qkLG22yf2Q6aBev64Lk6x R/pg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:in-reply-to:subject:message-id:date:cc:to :from:mime-version:content-transfer-encoding:content-disposition :arc-authentication-results; bh=ejRsUEoJyYpnf7lZFmc8xm43uyO6APnrwTo9F8oejSk=; b=OX2ELQdjs7s0YGnz4YDbcqPoov0Kgmfp9EmJYpRVUWQoG8p1vOWLSoyzu+ou3tvB11 u2dp6RhpSjurQyxDIRSetT66hX80mpkkQKOA0TD60cFcgR+d1NP2adREKhFlamvgHzxr /o20zaf8BEyVGY6TnYm0hdIUbP6ksAVr+xFT6VIVVdWc3A2C9LAWzOtHQzXAWsBqOfeZ g2T8rKP6LROlcvMYXqpLlyJIgCJi4O2JCURiLAA3zM3012WNCriu0uJmJ9fmRN8Na+J8 AVQ/GY4OcQ3YaXIQ3QbwJhse802o/UnUm/G5yesAxgnGWyyRSo6Myr2AMtDJUFIASyTF 9RXw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 68si1370271pff.141.2018.02.28.08.06.31; Wed, 28 Feb 2018 08:06:46 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934559AbeB1QFs (ORCPT + 99 others); Wed, 28 Feb 2018 11:05:48 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:34903 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932732AbeB1QFn (ORCPT ); Wed, 28 Feb 2018 11:05:43 -0500 Received: from [2a02:8011:400e:2:6f00:88c8:c921:d332] (helo=deadeye) by shadbolt.decadent.org.uk with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1er3Yt-0006XU-JL; Wed, 28 Feb 2018 15:22:31 +0000 Received: from ben by deadeye with local (Exim 4.90_1) (envelope-from ) id 1er3Yf-0008Ux-Jp; Wed, 28 Feb 2018 15:22:17 +0000 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Sinan Kaya" , "Vinod Koul" , "Shunyong Yang" , "Adam Wallis" Date: Wed, 28 Feb 2018 15:20:18 +0000 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.16 083/254] dmaengine: dmatest: move callback wait queue to thread context In-Reply-To: X-SA-Exim-Connect-IP: 2a02:8011:400e:2:6f00:88c8:c921:d332 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 3.16.55-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Adam Wallis commit 6f6a23a213be51728502b88741ba6a10cda2441d upstream. 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. 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 Signed-off-by: Vinod Koul Signed-off-by: Ben Hutchings --- drivers/dma/dmatest.c | 55 +++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 24 deletions(-) --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -148,6 +148,12 @@ MODULE_PARM_DESC(run, "Run the test (def #define PATTERN_OVERWRITE 0x20 #define PATTERN_COUNT_MASK 0x1f +/* 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; @@ -156,6 +162,8 @@ struct dmatest_thread { u8 **srcs; u8 **dsts; enum dma_transaction_type type; + wait_queue_head_t done_wait; + struct dmatest_done test_done; bool done; }; @@ -316,18 +324,25 @@ static unsigned int dmatest_verify(u8 ** 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) { struct dmatest_done *done = arg; - - done->done = true; - wake_up_all(done->wait); + struct dmatest_thread *thread = + container_of(arg, struct dmatest_thread, done_wait); + if (!thread->done) { + done->done = true; + wake_up_all(done->wait); + } else { + /* + * If thread->done, it means that this callback occurred + * after the parent thread has cleaned up. This can + * happen in the case that driver doesn't implement + * the terminate_all() functionality and a dma operation + * did not occur within the timeout period + */ + WARN(1, "dmatest: Kernel memory may be corrupted!!\n"); + } } static unsigned int min_odd(unsigned int x, unsigned int y) @@ -398,9 +413,8 @@ static unsigned long long dmatest_KBs(s6 */ 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; @@ -604,9 +618,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)) { @@ -619,21 +633,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); @@ -707,7 +712,7 @@ err_thread_type: 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; @@ -765,6 +770,8 @@ static int dmatest_add_threads(struct dm 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);