From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1E6A4369D59 for ; Thu, 11 Jun 2026 21:19:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781212756; cv=none; b=McxurQWsk4xkx45WwCs7WJfUDKnyOaCBPXXl5MY19MC1nUjHc2bbmVi5JT/aOyKHL7mSOhr+LhR/gh1afClv8dSkj90Gprma560ORXWm3zERujo4ZJ8sH2BTtjQ2EipxS8wMGLU8CFFTwdNNmdJ2ktRi8axCT/uF2tDa/vUV0ks= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781212756; c=relaxed/simple; bh=G0Y2kYFxPvABJSjJ0JAQZFuQtKeCecVEtrq4FJAfmRg=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=MNTJPRoZqfXfIjTfX/XHveg8NwccSm76o3AFH0of/oQnMt+URVkxFiNaaXAE+8rrnGlAyTyHI/IaQMpL8fRNlEaQh9JV/VcEa/AI3QdHuWL6wc23w2siYjKftHGPP29WQfyVg7uLoHitJIN/S+5uc0O3dD1igwWMbvt2gaFXgUk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=omM9EBTo; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="omM9EBTo" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 685191F000E9; Thu, 11 Jun 2026 21:19:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781212754; bh=fWY6UBoPChow4Ixdlbp9B0WKRPANk2syjkLhnFyE5rU=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=omM9EBToW7KLucg3j2hi9r1+/ZqWTEu8HXovvPh7FG6fu5RH7UfFVjQ9tSUsSsdlI k9LzobO2voD27fPgSeEoQOcclPg9rG2hjIaZ97l0dBirRmRgSuuKJpSi3deHy4hhtw NNLW0b/Ds3a49zofi646CAI2a6hq8i3MdjG5l5LrB3uOTUzv+T2xIVtwjukZeWOiiL 330XFiwRRqQR8v/Bhn+MG8QI7vJItoVyKdmrR/HoTIA0IUexjXVN4IdndVLbBl6hlp LEELHS6+Z34ANfTXsoPMA6os1j9GaErFvl9VdxfJ4hE3NaFYant2h8CjfITGOuYGCQ UJbjLTAujn6gA== From: sashiko-bot@kernel.org Subject: Re: [PATCHv2 9/9] dmaengine: mv_xor: add missing platform remove function Reply-To: sashiko-reviews@lists.linux.dev To: "Rosen Penev" Cc: dmaengine@vger.kernel.org, Frank.Li@kernel.org, vkoul@kernel.org In-Reply-To: <20260611210721.81979-10-rosenp@gmail.com> References: <20260611210721.81979-10-rosenp@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Thu, 11 Jun 2026 21:19:13 +0000 Message-Id: <20260611211914.685191F000E9@smtp.kernel.org> Precedence: bulk X-Mailing-List: dmaengine@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: New issues: - [High] Resource leaks in the newly introduced mv_xor_remove function: mis= sing clk_disable_unprepare, clk_put, and irq_dispose_mapping. Pre-existing issues: - [High] Incorrect teardown sequence in mv_xor_channel_remove leads to Use-= After-Free due to delayed free_irq and missing tasklet_kill. -- commit 9ff45f7ddd6e175cd19bc1dcad7a0c33d39a5914 Author: Rosen Penev dmaengine: mv_xor: add missing platform remove function The driver was missing a remove callback, so channels, DMA devices, and IRQs were never cleaned up on driver unbind. This patch implements mv_xor_remove to undo probe operations. > diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c > index 25ed61f1b0899..6065daec69ba9 100644 > --- a/drivers/dma/mv_xor.c > +++ b/drivers/dma/mv_xor.c > @@ -1460,8 +1460,19 @@ static int mv_xor_probe(struct platform_device *pd= ev) > return ret; > } > =20 > +static void mv_xor_remove(struct platform_device *pdev) > +{ > + struct mv_xor_device *xordev =3D platform_get_drvdata(pdev); > + int i; > + > + for (i =3D 0; i < MV_XOR_MAX_CHANNELS; i++) > + if (xordev->channels[i]) > + mv_xor_channel_remove(xordev->channels[i]); [Severity: High] Are we missing the teardown of the clock and IRQ mappings here? Looking at the error path in mv_xor_probe(), we have explicit cleanup for these resources: drivers/dma/mv_xor.c:mv_xor_probe() { ... err_channel_add: for (i =3D 0; i < MV_XOR_MAX_CHANNELS; i++) if (xordev->channels[i]) { mv_xor_channel_remove(xordev->channels[i]); if (pdev->dev.of_node) irq_dispose_mapping(xordev->channels[i]->irq); } if (!IS_ERR(xordev->clk)) { clk_disable_unprepare(xordev->clk); clk_put(xordev->clk); } ... } Does omitting clk_disable_unprepare(), clk_put(), and irq_dispose_mapping() in mv_xor_remove() leak the clock reference and IRQ descriptors when the driver is unbound? [Severity: High] This is a pre-existing issue, but calling mv_xor_channel_remove() here duri= ng unbind exposes a teardown sequence problem. In mv_xor_channel_remove(), dma_free_wc() is called before free_irq(), and there is no tasklet_kill() for the irq_tasklet: drivers/dma/mv_xor.c:mv_xor_channel_remove() { ... dma_free_wc(dev, MV_XOR_POOL_SIZE, mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool); ... free_irq(mv_chan->irq, mv_chan); return 0; } If an interrupt fires after dma_free_wc() but before free_irq(), the handler can schedule the irq_tasklet which will access the freed descriptor pool. Furthermore, without tasklet_kill(), a pending tasklet could execute after mv_chan is freed by devres, resulting in a use-after-free. Could this unbind sequence trigger these race conditions? > +} > + > static struct platform_driver mv_xor_driver =3D { > .probe =3D mv_xor_probe, > + .remove =3D mv_xor_remove, [ ... ] --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260611210721.8197= 9-10-rosenp@gmail.com?part=3D1