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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 52DC3C43219 for ; Thu, 24 Nov 2022 11:31:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230123AbiKXLbc (ORCPT ); Thu, 24 Nov 2022 06:31:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36622 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230177AbiKXLbS (ORCPT ); Thu, 24 Nov 2022 06:31:18 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D01176D4BB for ; Thu, 24 Nov 2022 03:31:17 -0800 (PST) Received: from kwepemi500012.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4NHwm4167HzRnsC; Thu, 24 Nov 2022 19:30:44 +0800 (CST) Received: from [10.67.110.176] (10.67.110.176) by kwepemi500012.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Thu, 24 Nov 2022 19:31:15 +0800 Subject: Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export To: Charan Teja Kalla , =?UTF-8?Q?Christian_K=c3=b6nig?= , "T.J. Mercier" CC: , Dan Carpenter , Pavan Kondeti , , , References: <20221117062152.3029018-1-cuigaosheng1@huawei.com> <99d3aee6-ba3e-5333-6f79-ddbcfc0e8843@amd.com> <2c9fa595-e788-5474-4f2b-ffbd08a70d13@amd.com> <83944425-c177-7918-bcde-9cf7296a613f@amd.com> From: cuigaosheng Message-ID: Date: Thu, 24 Nov 2022 19:31:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.110.176] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemi500012.china.huawei.com (7.221.188.12) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Thanks T.J and Christian, thanks everyone for taking time to review this patch. Charan, actually I don't have a good patch to to fix it, if you can submit a new patch to solve it, please feel free to do it. By the way, I'd appreciate it if you could send to me the new patch when you submit it. Thanks again! Gaosheng. On 2022/11/24 13:56, Charan Teja Kalla wrote: > Thanks T.J and Christian for the inputs. > > On 11/19/2022 7:00 PM, Christian König wrote: >>>     Yes, exactly that's the idea. >>> >>>     The only alternatives I can see would be to either move allocating >>>     the >>>     file and so completing the dma_buf initialization last again or just >>>     ignore errors from sysfs. >>> >>>     > If we still want to avoid calling dmabuf->ops->release(dmabuf) in >>>     > dma_buf_release like the comment says I guess we could use >>>     sysfs_entry >>>     > and ERR_PTR to flag that, otherwise it looks like we'd need a bit >>>     > somewhere. >>> >>>     No, this should be dropped as far as I can see. The sysfs cleanup >>>     code >>>     looks like it can handle not initialized kobj pointers. >>> >>> >>> Yeah there is also the null check in dma_buf_stats_teardown() that >>> would prevent it from running, but I understood the comment to be >>> referring to the release() dma_buf_ops call into the exporter which >>> comes right after the teardown call. That looks like it's preventing >>> the fput task work calling back into the exporter after the exporter >>> already got an error from dma_buf_export(). Otherwise the exporter >>> sees a release() for a buffer that it doesn't know about / thinks >>> shouldn't exist. So I could imagine an exporter trying to double free: >>> once for the failed dma_buf_export() call, and again when the >>> release() op is called later. >> >> Oh, very good point as well. Yeah, then creating the file should >> probably come last. >> > @Gaosheng: Could you please make these changes or you let me to do? > >> Regards, >> Christian. > . 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (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 F016EC433FE for ; Thu, 24 Nov 2022 11:31:25 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BFDDC10E6E7; Thu, 24 Nov 2022 11:31:24 +0000 (UTC) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by gabe.freedesktop.org (Postfix) with ESMTPS id 58FC710E6E7 for ; Thu, 24 Nov 2022 11:31:20 +0000 (UTC) Received: from kwepemi500012.china.huawei.com (unknown [172.30.72.54]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4NHwm4167HzRnsC; Thu, 24 Nov 2022 19:30:44 +0800 (CST) Received: from [10.67.110.176] (10.67.110.176) by kwepemi500012.china.huawei.com (7.221.188.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Thu, 24 Nov 2022 19:31:15 +0800 Subject: Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export To: Charan Teja Kalla , =?UTF-8?Q?Christian_K=c3=b6nig?= , "T.J. Mercier" References: <20221117062152.3029018-1-cuigaosheng1@huawei.com> <99d3aee6-ba3e-5333-6f79-ddbcfc0e8843@amd.com> <2c9fa595-e788-5474-4f2b-ffbd08a70d13@amd.com> <83944425-c177-7918-bcde-9cf7296a613f@amd.com> From: cuigaosheng Message-ID: Date: Thu, 24 Nov 2022 19:31:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.110.176] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To kwepemi500012.china.huawei.com (7.221.188.12) X-CFilter-Loop: Reflected X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Pavan Kondeti , dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, sumit.semwal@linaro.org, Dan Carpenter , linux-media@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" Thanks T.J and Christian, thanks everyone for taking time to review this patch. Charan, actually I don't have a good patch to to fix it, if you can submit a new patch to solve it, please feel free to do it. By the way, I'd appreciate it if you could send to me the new patch when you submit it. Thanks again! Gaosheng. On 2022/11/24 13:56, Charan Teja Kalla wrote: > Thanks T.J and Christian for the inputs. > > On 11/19/2022 7:00 PM, Christian König wrote: >>>     Yes, exactly that's the idea. >>> >>>     The only alternatives I can see would be to either move allocating >>>     the >>>     file and so completing the dma_buf initialization last again or just >>>     ignore errors from sysfs. >>> >>>     > If we still want to avoid calling dmabuf->ops->release(dmabuf) in >>>     > dma_buf_release like the comment says I guess we could use >>>     sysfs_entry >>>     > and ERR_PTR to flag that, otherwise it looks like we'd need a bit >>>     > somewhere. >>> >>>     No, this should be dropped as far as I can see. The sysfs cleanup >>>     code >>>     looks like it can handle not initialized kobj pointers. >>> >>> >>> Yeah there is also the null check in dma_buf_stats_teardown() that >>> would prevent it from running, but I understood the comment to be >>> referring to the release() dma_buf_ops call into the exporter which >>> comes right after the teardown call. That looks like it's preventing >>> the fput task work calling back into the exporter after the exporter >>> already got an error from dma_buf_export(). Otherwise the exporter >>> sees a release() for a buffer that it doesn't know about / thinks >>> shouldn't exist. So I could imagine an exporter trying to double free: >>> once for the failed dma_buf_export() call, and again when the >>> release() op is called later. >> >> Oh, very good point as well. Yeah, then creating the file should >> probably come last. >> > @Gaosheng: Could you please make these changes or you let me to do? > >> Regards, >> Christian. > .