From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 520F5C77B73 for ; Wed, 31 May 2023 08:19:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=5TVk5OMZyOeynk6TXGtEgpWdfLaLXgGl6ilxqXMsVFw=; b=U8svqgh+12xNGk B/Bpn6nW89WzXi2/UWzGbP4QVIaI4Yjk6gx7fjsBh5+cNBKEIxxtnHH1nvocND7PHth9y6uB8EGRM KPzNL9t25KnFN35d4yWKiHyxiwkBci8BY13lbPGsC0D+0T1nZN1PvIgaNoF/maF+9nniu7G6VG+4R N2tskQMJtLtPKklnO5zB+bEqDbX/Rhupl+xwiAimEhqJOoYOyMFEY+1EdMLE5yDh6KGnU6fmX6EpY Cb+jRclvGXCnOz2ZN6ON6SUZQFbANqWrlt538sqz5t42x+q61mbqC5FHW7dt+1ZvGcZ2X4V4gpwD4 vKv5bJVgDfHSUJM6rh6Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q4H3E-00Gaxe-0P; Wed, 31 May 2023 08:19:24 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q4H3C-00Gax7-0H for linux-arm-kernel@lists.infradead.org; Wed, 31 May 2023 08:19:23 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id CA97B62B31; Wed, 31 May 2023 08:19:20 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id C410FC433D2; Wed, 31 May 2023 08:19:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685521160; bh=0HCQtl+r90qoo8vag/j4fmBsYtYN/bLTT76larf8Zh8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AXU+m6wUmO04i+P8FRRsQWmSDFaUGpTI8wLkyT496Wm6nPchKPS5Wa32z/mjLweic LWBL1gR289uyb9YbVtDBEkogCpNM0xV+41uYlwQcUZml2zZjU7TNN2Q7j0SuMyU8wQ Sd4SrG/Q/EiVAVthWxdfFO1WNbCwjVpFgJIatnMPfQ3eTnxALJvX2R4t2nQyqQgSW+ nNH8cpv+2hFu510vTfmm+eK2ZohnviIo2+A1Rpo36HH6G4Ar2zlBugpwHHT/FlONa4 vZvIG/2bebOosP2ljYuo+8dAk64rnFCLL7Ns5rU0blWMFDu9vwY4RXQGTDCCFoFKGc S192en2LAOZEQ== Date: Wed, 31 May 2023 10:19:17 +0200 From: Andi Shyti To: lm0963 Cc: inki.dae@samsung.com, sw0312.kim@samsung.com, kyungmin.park@samsung.com, airlied@gmail.com, daniel@ffwll.ch, krzysztof.kozlowski@linaro.org, alim.akhtar@samsung.com, dri-devel@lists.freedesktop.org, linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/exynos: fix race condition UAF in exynos_g2d_exec_ioctl Message-ID: <20230531081917.grx3qqqm7usaqoa5@intel.intel> References: <20230526130131.16521-1-lm0963hack@gmail.com> <20230530222150.24oogloda6wtvpvm@intel.intel> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230531_011922_166005_77FBD08B X-CRM114-Status: GOOD ( 26.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Min, > > > If it is async, runqueue_node is freed in g2d_runqueue_worker on another > > > worker thread. So in extreme cases, if g2d_runqueue_worker runs first, and > > > then executes the following if statement, there will be use-after-free. > > > > > > Signed-off-by: Min Li > > > --- > > > drivers/gpu/drm/exynos/exynos_drm_g2d.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c > > > index ec784e58da5c..414e585ec7dd 100644 > > > --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c > > > +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c > > > @@ -1335,7 +1335,7 @@ int exynos_g2d_exec_ioctl(struct drm_device *drm_dev, void *data, > > > /* Let the runqueue know that there is work to do. */ > > > queue_work(g2d->g2d_workq, &g2d->runqueue_work); > > > > > > - if (runqueue_node->async) > > > + if (req->async) > > > > did you actually hit this? If you did, then the fix is not OK. > > No, I didn't actually hit this. I found it through code review. This > is only a theoretical issue that can only be triggered in extreme > cases. first of all runqueue is used again two lines below this, which means that if you don't hit the uaf here you will hit it immediately after. Second, if runqueue is freed, than we need to remove the part where it's freed because it doesn't make sense to free runqueue at this stage. Finally, can you elaborate on the code review that you did so that we all understand it? Andi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel