From patchwork Wed Jan 11 13:00:21 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: iari@itu.dk X-Patchwork-Id: 9510039 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 E144B601E7 for ; Wed, 11 Jan 2017 13:00:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D3B1928613 for ; Wed, 11 Jan 2017 13:00:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C77D928620; Wed, 11 Jan 2017 13:00:41 +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=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8E66028613 for ; Wed, 11 Jan 2017 13:00:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936781AbdAKNAk (ORCPT ); Wed, 11 Jan 2017 08:00:40 -0500 Received: from mail-eopbgr50102.outbound.protection.outlook.com ([40.107.5.102]:45027 "EHLO EUR03-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933649AbdAKNAi (ORCPT ); Wed, 11 Jan 2017 08:00:38 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ituniversity.onmicrosoft.com; s=selector1-itu-dk; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=ck0OJcvB0171H9IandJa9688TTULtUwq/9fBJi7u1AI=; b=b9GTlJXIgDOPYkGhGptkfcEWME/3a9h6khovzdE4mAEqkyN7XYa06NPEuJqzi4NaG99Sqi8JiVfT2Iv8K4nZ22AayjbHSjfkf7OC41Vkk62K9EJZHOmwdmCfYyNjXmUx/IeWFhujkrxzoGhv5yjTGI7cIdwD2AT8Ij4A/qkkddg= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=iari@itu.dk; Received: from hp.itu.dk (130.226.132.19) by AM5PR0202MB2706.eurprd02.prod.outlook.com (10.173.90.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.845.12; Wed, 11 Jan 2017 13:00:34 +0000 From: To: Marek Szyprowski CC: Bartlomiej Zolnierkiewicz , Vinod Koul , Krzysztof Kozlowski , Dan Williams , , , Iago Abal Subject: [PATCH] dmaengine: pl330: fix double lock Date: Wed, 11 Jan 2017 14:00:21 +0100 Message-ID: <1484139621-18706-1-git-send-email-iari@itu.dk> X-Mailer: git-send-email 1.9.1 MIME-Version: 1.0 X-Originating-IP: [130.226.132.19] X-ClientProxiedBy: AM5PR0502CA0013.eurprd05.prod.outlook.com (10.175.37.151) To AM5PR0202MB2706.eurprd02.prod.outlook.com (10.173.90.14) X-MS-Office365-Filtering-Correlation-Id: f252e0cd-a9f9-4dc0-f99d-08d43a21d4fe X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001); SRVR:AM5PR0202MB2706; X-Microsoft-Exchange-Diagnostics: 1; AM5PR0202MB2706; 3:WfBAUPMQ5IyEAIa8/lsUUt1s6HhCB+2SAz01jDKeRhpCzDJaYy5/U7sNzcgN17Ks/qcfJE4DeEuF5iKXrta08X4xcnsXFrX0NuNzzHqzZ5212/63b5c0ZPjbvRAaKjuPTp4X4bxb4ousOaYLxXNaF8bVPUC+IhAEZJ/PmqNamIfIs2dMNUY9GHLxDMa/iR4apDtlYOy6lGl6ZllDMGcVcx2qZlF6+SQ7CFyKCwM2rGATiI3ivuPMCsmpI979eD4wsfa8BkvrIbj6nsK7Kfuw7g==; 25:aDLSkIJE7d3iaz8QGmC2EcFOQCXXAcWvzZSn8XvVEwEMEowC32PWZ/sN0sdesrQCmZgSRni/8fV2Hcuc/oWiwQua98vCkZENmWge/z3dTzJ7wr6+5S2CIzcqEKZptltEPe0E92UdMp3gnfiE/Tl/dBJFu0F205tNNUzRIa8YqcrH32gRRUHci/Nt4w6PGJELYjZ2ihvPDpvbe23JtKrtLB8+Oqd3XMK6JW6P9zEv0qyahihAzOvw+XGImB3FuXGfHCRD85E14i9bZGpjKLLOpcfYl720dPOrfOJK1ZtlXxba/n+eeHiwkF4P08H3Rny1OnHkGJ9lL8giHdv8mUofLnZkJtiAfaoLCYZy0x60KfeF2100XWGK20xzGdhU+Hhj+KqdcY9t2p1LojTEduaAQ+vkbdIbhd8TqFXbzJLNx0Uw6seAkQdut6/Oo5wa569FFxGtYTbvdCY1WpVF4PlpdQ== X-Microsoft-Exchange-Diagnostics: 1; AM5PR0202MB2706; 31:gv3Pqn6kAMpfLupvDcyNeU6vTz3ZYMUKqgJTYq/NxFFOIa//obNSiKfy4ycBSBQZvq1My4zxQca02WLFVjXKrxGuHuzM5wbfqVLD6EheqWqwIsL/YgFf32FNqp84phh2x9cvKNEtb/6HQn+pVq6hJ+i93g/WBunzT15hyaXgjiRX4NqU+/4PfnkggL9dPcSyLQexmbsGM/zIeIksSPBQw80nbIsIbJarhA4GgILOdhb4bYPp8XZbDmtdRYaX4K5N4a5drq4FW3WGgR7GrPUfyg==; 20:ZqtWMi+TrtEOD/ySlElAi8JiciLu1e6LpSRoY7gW0mSy4eS6vILD5zm08yVcQxCduzp2oSWO/OQf7cpyokPaVq/RSzgXipFx9QHJeegV64uRQ5TuHOvpx8Tz+/0mRVAq+XTqtTTYAMiS0E7YtbuDVdEZ46WvvxN1XeZjmv9pEcRUKABnGnsm3SXx+s/At3oXz1y3RfAyiZ5cuGNEvB6vlfrmptqcD7oGp0pLPQpI4kc9uonhAONOKwHhBMEURj7VTSJo8ssl5og3DZRpSvm8NOnsG93Ar/BZMAAGoL7iE3ne4+B+qAzk7udez7jy3cZQ64C0uVymUHMn6pTGkVrH5jEe2jWqGm8oDlSYWHp7thvhjYBeyUAg9VOi0tPAv6p4ysqwYlFRW1WmAzx6tRUPpdz1um9C4rbD69nIZdQ6eRjUcvHBRJod6qc3sqBrwOUewm3SYrD0jQKAOpsm5a5pqAVUIh7tshPe4JDTNMVINhe+oUKwtmg8tzeoZcjwkHcMxgGf9TIPReHf7HZUjvIfZl9HTfKtkFDKbjvAntJqeSFME8hrJAfAb0j69h+ygDTTS+ScfdNdXQ2kdSLtwmOTD/Rkqv+GaLKE192O4WVOebM= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6041248)(20161123562025)(20161123555025)(20161123564025)(20161123560025)(6072148); SRVR:AM5PR0202MB2706; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0202MB2706; X-Microsoft-Exchange-Diagnostics: 1; AM5PR0202MB2706; 4:g1IlJ6wjXMR9cudeDjygdTSl1mmyfQJmksl4nhEQxJocDHxmXkatp/CI5qpS+HSiJaGPQhMdh1wbXAtIotz6hjPJsv17Ph0UBAlbYRU8WxETBl9UL6T4MSla0AYowG2bV1qNwkTYcVCmmIklt8duiJJyDdHjQYLzwGAk4DndbYPsn5niQ/G31JV072Li6rgeh8t/VppUl6VM8G3aEFG0d+TBF6cwJ6JTM1H1iRtIsKd6PWYj49o5jI9veDVWLA+C7InVY/zDf2BdpaOciriaSYgZW12t7NQOISWcRYWgfNkrXw90hR8O4SMSb8B3UkYHqKU3oYBKo3IumuzoO5YquAmU/OcSijEXQPKr/vOi85GYiLxirscaKh28Q13oIVJfBoPjX6gCVkYnqvEJbLFy8dmrq6JTLA8jCaXWPfdpbSEkYRI8buWzOw7HbPuDdCvf5906OsBy60eMPM68SJsWBa4NSZ1f1QfdXveGzAjU65aFWAGeQ0yCbuUVSKAfn+BylAEszwZLuWpfzV6GTno1kgwmNkw5YKc6dGvKt6jGCiFZHPM+/yEBtHUoIf07uNAd X-Forefront-PRVS: 01842C458A X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10019020)(4630300001)(6009001)(7916002)(39450400003)(199003)(189002)(5003940100001)(38730400001)(42186005)(189998001)(33646002)(2906002)(86362001)(101416001)(6486002)(25786008)(6916009)(6666003)(68736007)(50466002)(48376002)(97736004)(53416004)(47776003)(92566002)(66066001)(50986999)(2876002)(4326007)(110136003)(50226002)(105586002)(36756003)(106356001)(8976002)(8676002)(86152003)(6306002)(54906002)(81156014)(3846002)(6116002)(81166006)(5660300001)(74482002)(7736002)(305945005); DIR:OUT; SFP:1102; SCL:1; SRVR:AM5PR0202MB2706; H:hp.itu.dk; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: itu.dk does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM5PR0202MB2706; 23:CmC9qoKfc8ppsItTmXUBvbm6YXvY/EzF//yys6R?= =?us-ascii?Q?/8sfrVrwOFWsFAs8FqL3ds51EWIfj5B+Y3yp5QBYmtzJtQfCvkoa/N3xwScF?= =?us-ascii?Q?oaHRTWfpcVUjOW2d7DRCINS17yqvo+eMHTfRCDqrkTZT3xb+93MnpGnCtTM5?= =?us-ascii?Q?nv2dJTTuTZc1bQK4b7CPb7DCW+gFVgSz5ADVBz3//AyDN+AdzsmLx7qJcPi1?= =?us-ascii?Q?vK8f73YKro/pk1SaqsqUZZigboPiRwD23+YM8jPmOZc53FrjfKCnIfM4e0UE?= =?us-ascii?Q?43XOgs4sVogwALBBagHBLKkIvzSwtE7xWkn78DwSEVzoJgoWrLd7kS1ssLlK?= =?us-ascii?Q?IZLDxf3lnpJ417GkjBc9NLSimWgjsYZdq/EbcdUuJkG4EwgO4O5xKPPkmIZ/?= =?us-ascii?Q?Jeaz67fWgcWg1fA6A9gN5GqzH78G9bLR/fA09ZodsFKzCVxv0qod0d8lT+0T?= =?us-ascii?Q?xNyx/F1Yp/xIlGgVWhS3rKEaNL/1bwTUYDXp3/1784Q9gWjzy4bQkJ4DORv/?= =?us-ascii?Q?tvms1WZtHkY2vYXeNqI1jhiAFdjXFxxBz1w+YG8z32kd6qabvtFwEtneg0/e?= =?us-ascii?Q?dEuR5tDX3GlE/C2AxUqwrmuLJne0hUTws+/wKDVKYWGDNZuS4JeaMxP5l6oL?= =?us-ascii?Q?GrKjDwg+MfbifpMP3hZK8ekcDrJ6UVs3F4vE3TfhxbQ8RkmNogWfz02u7Dq/?= =?us-ascii?Q?FitBFNFGo8D3PmRXb688txcxX87GJBffv4c+8KGuu/b4RotAsD4i6jh6EJiy?= =?us-ascii?Q?IjxFDU9VrOckBmSrajPRRlXLys7TZ0WNevAmnuqLdAo4b1MQTvwALpfESURG?= =?us-ascii?Q?wXEV6cbDyNgm2yRx0gvzH3Qh4lZ2VV+T4VoniR8V/h8Q9xdUk5aVPrXrqgMD?= =?us-ascii?Q?2vvy8vSHPwF9RIFt7m0umKdVH4BrTpJyr9NfkxfpC9MYKSivu2N3LE8wqKkJ?= =?us-ascii?Q?AgjopNojibS2EC3Lvgv6ZojuAS2ZD32BbFAAZN05NSbiv88EBNRlqNhJhGdN?= =?us-ascii?Q?17qAZrh1X5YJxbaJpQBkU05vC78vyxBcTH4926pMafb5ezIVtOvNGb9cQiHv?= =?us-ascii?Q?wWOAzeXkSpCZk1Gfmn1NCvnkBatwWzAmVuK4/2bvn0U9oAAGoenMtewjs0Y+?= =?us-ascii?Q?7D0FtyU7Yj3TEYszfrSSY/IgvpPZdv51A?= X-Microsoft-Exchange-Diagnostics: 1; AM5PR0202MB2706; 6:stxohNrW3hTxg+1+plgWQsuVVIS7M6ZccgZMYrIt2OqEa7CqbV2evqkWRuOP2HPXMmYPzq4ZN+jDYX3hduRYktT2gU0MUqfQMUipVYs5P0tHiU98oxRm+tf/Ai+zbj6i19atdJf2MAKDCqSXP5bwsQPtFQ+sc6PwdQkO2hG2UAGJPOIxYyH1vrqNkdIG/cZ3U1ZSBv7DI27LncxbL2ofUzLgZHyhxyKgI0N4+OxkUjZq717O3VJl17fkM6FXnVII/7/MkGWp0tEinFZKox/oUbzux1JINwke8wSslAKov+rm8vm++3Na4MXI1WaUybg05/n/C60fjSKqZs9MN8q43LfDoJZHLsK8396aowNaEP1qSiMEru4BuZsLZDsGfNlr5Mshkmw46cFQES0lOlGIOwSfshxTtQKeTorgz+DRQvw=; 5:tEUYNy85/Q8Grg6WEZbdMuzrSHyBYTcbMgeGcgizTXCFIjiOL99by1vNR+fMcDO/kWQLX2SPcEKkXhTb6IUjH5p2WUVN6cJIA7XhLZ6rXxLbU1U9tKZksCSUKQFuBUJML0QH9fSeaoX4PHzBK8IZLw==; 24:cpXTFaAbZ3Tf6qHoz3HgJzvfxLJeo4gcZXZLJWNrHViVzWfBAyS92VpU55tdV2R1H8JOzZHn7qB/17KDa0Ow6f/BM2cb5cudYWu20muF1bM= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; AM5PR0202MB2706; 7:vCMYYs034xcO0ieYwgMpmGqGkCxDPqN7SEO+a0U1DsmMhKwKh2OkRSRVSFKSdARXKhOILvjcwb7uuJWMEbf2yau0RX4JZhU9wqb4rLnXv+XcoAboi0ltz1djxdE9D8n1BjvGEoYfxtJDxqjnM4spviX6fbuJlCfuajzMc66ZXouUA8tq+6gY4Fxl+1qqDES5EAe/uncsqKz4P3DSxsGJfecW7OfchHbc3DN1wT0Jcf13cNqjgBkXhe1L3yu6dhCUH+OK1R4NTAran+B2Q19sVmpDgwPbF0lL9LrVLyFNjFD9aVO5SNMPmqUrum5hIocIsfMxgOywapdrJYHXiZVK6VdKPRh51fYarx1i8ayAad7WDs88TajgwSmrKWkvgDS3rLOELFa/34gT1oKuK2Ze3lhjXpjgknZY7YEbrJAPOFG6Kj7PM+aJjcFV5QkeDpj98WhU5hrQCbTjs8jKMxm4Qw== X-OriginatorOrg: itu.dk X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jan 2017 13:00:34.5612 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0202MB2706 Sender: dmaengine-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: dmaengine@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Iago Abal The static bug finder EBA (http://www.iagoabal.eu/eba/) reported the following double-lock bug: Double lock: 1. spin_lock_irqsave(pch->lock, flags) at pl330_free_chan_resources:2236; 2. call to function `pl330_release_channel' immediately after; 3. call to function `dma_pl330_rqcb' in line 1753; 4. spin_lock_irqsave(pch->lock, flags) at dma_pl330_rqcb:1505. I have fixed it as suggested by Marek Szyprowski. First, I have replaced `pch->lock' with `pl330->lock' in functions `pl330_alloc_chan_resources' and `pl330_free_chan_resources'. This avoids the double-lock by acquiring a different lock than `dma_pl330_rqcb'. NOTE that, as a result, `pl330_free_chan_resources' executes `list_splice_tail_init' on `pch->work_list' under lock `pl330->lock', whereas in the rest of the code `pch->work_list' is protected by `pch->lock'. I don't know if this may cause race conditions. Similarly `pch->cyclic' is written by `pl330_alloc_chan_resources' under `pl330->lock' but read by `pl330_tx_submit' under `pch->lock'. Second, I have removed locking from `pl330_request_channel' and `pl330_release_channel' functions. Function `pl330_request_channel' is only called from `pl330_alloc_chan_resources', so the lock is already held. Function `pl330_release_channel' is called from `pl330_free_chan_resources', which already holds the lock, and from `pl330_del'. Function `pl330_del' is called in an error path of `pl330_probe' and at the end of `pl330_remove', but I assume that there cannot be concurrent accesses to the protected data at those points. Signed-off-by: Iago Abal Reviewed-by: Marek Szyprowski --- drivers/dma/pl330.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c index 87fd015..3370e56 100644 --- a/drivers/dma/pl330.c +++ b/drivers/dma/pl330.c @@ -1696,7 +1696,6 @@ static bool _chan_ns(const struct pl330_dmac *pl330, int i) static struct pl330_thread *pl330_request_channel(struct pl330_dmac *pl330) { struct pl330_thread *thrd = NULL; - unsigned long flags; int chans, i; if (pl330->state == DYING) @@ -1704,8 +1703,6 @@ static struct pl330_thread *pl330_request_channel(struct pl330_dmac *pl330) chans = pl330->pcfg.num_chan; - spin_lock_irqsave(&pl330->lock, flags); - for (i = 0; i < chans; i++) { thrd = &pl330->channels[i]; if ((thrd->free) && (!_manager_ns(thrd) || @@ -1723,8 +1720,6 @@ static struct pl330_thread *pl330_request_channel(struct pl330_dmac *pl330) thrd = NULL; } - spin_unlock_irqrestore(&pl330->lock, flags); - return thrd; } @@ -1742,7 +1737,6 @@ static inline void _free_event(struct pl330_thread *thrd, int ev) static void pl330_release_channel(struct pl330_thread *thrd) { struct pl330_dmac *pl330; - unsigned long flags; if (!thrd || thrd->free) return; @@ -1754,10 +1748,8 @@ static void pl330_release_channel(struct pl330_thread *thrd) pl330 = thrd->dmac; - spin_lock_irqsave(&pl330->lock, flags); _free_event(thrd, thrd->ev); thrd->free = true; - spin_unlock_irqrestore(&pl330->lock, flags); } /* Initialize the structure for PL330 configuration, that can be used @@ -2117,20 +2109,20 @@ static int pl330_alloc_chan_resources(struct dma_chan *chan) struct pl330_dmac *pl330 = pch->dmac; unsigned long flags; - spin_lock_irqsave(&pch->lock, flags); + spin_lock_irqsave(&pl330->lock, flags); dma_cookie_init(chan); pch->cyclic = false; pch->thread = pl330_request_channel(pl330); if (!pch->thread) { - spin_unlock_irqrestore(&pch->lock, flags); + spin_unlock_irqrestore(&pl330->lock, flags); return -ENOMEM; } tasklet_init(&pch->task, pl330_tasklet, (unsigned long) pch); - spin_unlock_irqrestore(&pch->lock, flags); + spin_unlock_irqrestore(&pl330->lock, flags); return 1; } @@ -2228,12 +2220,13 @@ static int pl330_pause(struct dma_chan *chan) static void pl330_free_chan_resources(struct dma_chan *chan) { struct dma_pl330_chan *pch = to_pchan(chan); + struct pl330_dmac *pl330 = pch->dmac; unsigned long flags; tasklet_kill(&pch->task); pm_runtime_get_sync(pch->dmac->ddma.dev); - spin_lock_irqsave(&pch->lock, flags); + spin_lock_irqsave(&pl330->lock, flags); pl330_release_channel(pch->thread); pch->thread = NULL; @@ -2241,7 +2234,7 @@ static void pl330_free_chan_resources(struct dma_chan *chan) if (pch->cyclic) list_splice_tail_init(&pch->work_list, &pch->dmac->desc_pool); - spin_unlock_irqrestore(&pch->lock, flags); + spin_unlock_irqrestore(&pl330->lock, flags); pm_runtime_mark_last_busy(pch->dmac->ddma.dev); pm_runtime_put_autosuspend(pch->dmac->ddma.dev); }