Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 13 Jul 2021 12:24:10 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Jacob Chen <jacob-chen@iotwrt.com>
Cc:     Ezequiel Garcia <ezequiel@collabora.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Heiko Stuebner <heiko@sntech.de>, linux-media@vger.kernel.org,
        linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
        kernel-janitors@vger.kernel.org,
        Dongliang Mu <mudongliangabcd@gmail.com>
Subject: [PATCH v2] media: rockchip/rga: fix error handling in probe
Message-ID: <YO1burFzVK+l3JST@mwanda>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from mwanda (2a02:6900:8208:1848::16ae) by LO2P265CA0050.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:60::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4308.21 via Frontend Transport; Tue, 13 Jul 2021 09:24:17 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 13570910-4a0a-4570-16ad-08d945dfff95
X-MS-TrafficTypeDiagnostic: MWHPR1001MB2159:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <MWHPR1001MB2159E45C429400F45068E34C8E149@MWHPR1001MB2159.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:1850;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: BpnS7Pn6XEBufSdYBF6vjWlBY4YT0yJ52ld4tIEkn/a+VfTZxZpvmMHGJxOU70PuOVReHKPLRxdMOykw8g6CdvzmC2wMppg1tB4/vtaSHOr25CelCEBPXe2x2Jil4nCeJw6/JKa7Z5kPH63jjrRlryH+ONJTFXg1VjHWNCtJfOS7rbGMkiwrFgfsl1WkjO1poiojmie1aRFdv4aw2GwyG6NE/SMN4g/9HhowROLIgwv0XJX12UQj+UysoOmMJ88gNyaM57iPaG8wBSiKJLh2P5+xOCAJG5x/D3G2jPgKFMieOUnx950AmJAhlP2xzXytu7T68R6Qz6ZWVJt3dcz8AWm2Q/NqnhOmYgy/gaiUrciqK7yfwdvV6YK0BmRO/JCHCZHar0+ZM181chfErbXMiM8uWBfNQ+FwqjS+oAeZW/sPqLZGvs9HhwH3eqmEtuKDD8jVw+zadotogDBvV7DRQul8M0TQcBxQTB3LC7JwrZKDVrlLj8KcTQceDGch8wsiG6lyNfUJ1tTOmOzOFDFk4BxH/PpqITX5Eh4KPjKYtmyuruq3takUAuQkO4LIaZrGeBi23al6ZK0okdAD+qWBcz1g55XPH9bCsfLUqtMCstvFPjwZA6wktrn06Rjet9Zm+PFwPnYFxwzmIdsLeKoXBBNMF5vH0Vu9A7VOzyMNOA70SVzgX0NvYDPafqHf737+GcU2LSzBlgBUHIC6NE/bhg==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR1001MB2365.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(396003)(346002)(39860400002)(136003)(366004)(44832011)(38100700002)(83380400001)(9576002)(5660300002)(6666004)(33716001)(316002)(54906003)(86362001)(52116002)(6496006)(9686003)(2906002)(55016002)(6916009)(186003)(8676002)(66556008)(66476007)(66946007)(4326008)(478600001)(8936002)(67856001);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?hRDovjINpn75+Ic5O3QmuQN9OMGSKgweBQnH+j6DG1grsMMbeJm6k/1TtFbX?=
 =?us-ascii?Q?+rXjS0vFDE/thfg9klW+w72kpQ55Me940WirCa/TXKYu4HneeV7L0D40c0wG?=
 =?us-ascii?Q?u7FrImz+1ja0ESIBrlm3ZEaoyoxp/pn3p393ldb6R8PcIXrysrp+9M6KVg1n?=
 =?us-ascii?Q?fqqvROTZxicRaI07fie9X+C8W1EiPSvjBjFdWb4649aqOZ4BuQsvvU2VAf56?=
 =?us-ascii?Q?6Kt01gOsfT86bymrAX7Y5Bs6UIjnikFQxx0qcpuafA/Sj+laR9NIsblLd0cb?=
 =?us-ascii?Q?zaaIEOM1Knsea8P4WZA+HJM+6MFYPmqg3NOsMY0LI5wJIauVT41M2bq+Rx0F?=
 =?us-ascii?Q?2KFMZchb+XusP3iQgUpLpHjwTvVTTzBraYrQ5SlS5bBdteM3rpZpb+C9ehiM?=
 =?us-ascii?Q?gi02JJpr8kc/YP4Gv8JpE0Qvx9wJ43upCDKeJWmraIPZbpSPAqCadiW1Sveh?=
 =?us-ascii?Q?5Cnaya3Wrd03fYMRTzt6q13TN97Xbgk4ll5ayuGpvdxeJcMlEmiT1hIBo2ak?=
 =?us-ascii?Q?Yz0xLno87RozuaBIiMlqFJatOtiLaK9oy8ukTKdGRZPu0Ui4eVtXHzJHHV0i?=
 =?us-ascii?Q?VFbjMM93tvIJg54sSFeLJ+7IkfTuR7KAnW3/M2+aq4CTqqpVw6rRojPjp0QE?=
 =?us-ascii?Q?MC41McsTxT25FBypQARNqHOpj0EQrfbftMxmYYrNTj6kAESPdTjgMTrvoCrh?=
 =?us-ascii?Q?gqmgXTIJYH9XKlULdehNy+IaNaIFxx5nVP4CmHqYkhyEGO91aPh3Fg7Yi3Cq?=
 =?us-ascii?Q?Mp870SkXXAy5SVh6BJjBmH+oa13MWJJZRVKkhpDZudU5jNcuRMBKiteCEWNl?=
 =?us-ascii?Q?7EvJN0EmwO7PuQVj8kETjuTqcJH1xM6wio2S6yhmB1pY81fu6CExE/hdbFqh?=
 =?us-ascii?Q?xhMBkhvqfkdlyn2e6/DHbvq9vIGha3d5W6lFlQreiPLTHuEpFPA597aOXOqO?=
 =?us-ascii?Q?yR9rq0KX9ArWKwC6amVp3DYDCG1utUSr8V9fxapCHARW70ibiYGFXrWfZ+gM?=
 =?us-ascii?Q?qDWbPj3k+VfQeq+mRtVuA6hZbuI4Dja3EyTEKk5b1QVJepjnCcekXuCWawoY?=
 =?us-ascii?Q?/oRHKyPlrKzDdVrz0EgpWYMvZyeyYjscAkRxjYaqK8j6qCo7IqXixIV38U7F?=
 =?us-ascii?Q?zslV5pNedUrNYXPB6XvyXUYHXEJBKhpT15q28FL9GPA0Me4Ua13O3lLWFslW?=
 =?us-ascii?Q?Jg3RqyxvQzGdMmAhjoH5bb96Rkr3RKNcp852yihmW6OWC4i783CMGU5HK0Px?=
 =?us-ascii?Q?KZK+umdRsVJl9JF/KDRMwMDiq3HkKHyhiLU8CMMDb8UQ+pgYFMyHbQlgDpRZ?=
 =?us-ascii?Q?udzj09BaruchtxVb6Hmo81eYXIF88neopm98oPqUlVh4xlbUIXM8nMP8iUGd?=
 =?us-ascii?Q?TqHIQAU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 13570910-4a0a-4570-16ad-08d945dfff95
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Jul 2021 09:24:22.2587
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: l/SM9UMdUPYK+ROBFthqZi5OZpyyP8yNFIrdjsNhFNpKB/cM3dfj+vfqzV0kuod+xUGwdy7KC5dKZLqj2Oy1tnfx4tqYdzc8XRCSccXazmc=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR1001MB2159
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10043 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 malwarescore=0
 spamscore=0 suspectscore=0 adultscore=0 mlxlogscore=999 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2107130058
X-Proofpoint-GUID: 79YkD6Rc0_MYJBcWtfG0UeIBRmDHBRd5
X-Proofpoint-ORIG-GUID: 79YkD6Rc0_MYJBcWtfG0UeIBRmDHBRd5
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

There are a few bugs in this code.  1)  No checks for whether
dma_alloc_attrs() or __get_free_pages() failed.  2)  If
video_register_device() fails it doesn't clean up the dma attrs or the
free pages.  3)  The video_device_release() function frees "vfd" which
leads to a use after free on the next line.  The call to
video_unregister_device() is not required so I have just removed that.

Fixes: f7e7b48e6d79 ("[media] rockchip/rga: v4l2 m2m support")
Reported-by: Dongliang Mu <mudongliangabcd@gmail.com>
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  The v1 patch only fixed the use after free and not the other bugs

 drivers/media/platform/rockchip/rga/rga.c | 27 ++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
index bf3fd71ec3af..6759091b15e0 100644
--- a/drivers/media/platform/rockchip/rga/rga.c
+++ b/drivers/media/platform/rockchip/rga/rga.c
@@ -863,12 +863,12 @@ static int rga_probe(struct platform_device *pdev)
 	if (IS_ERR(rga->m2m_dev)) {
 		v4l2_err(&rga->v4l2_dev, "Failed to init mem2mem device\n");
 		ret = PTR_ERR(rga->m2m_dev);
-		goto unreg_video_dev;
+		goto rel_vdev;
 	}
 
 	ret = pm_runtime_resume_and_get(rga->dev);
 	if (ret < 0)
-		goto unreg_video_dev;
+		goto rel_vdev;
 
 	rga->version.major = (rga_read(rga, RGA_VERSION_INFO) >> 24) & 0xFF;
 	rga->version.minor = (rga_read(rga, RGA_VERSION_INFO) >> 20) & 0x0F;
@@ -882,11 +882,23 @@ static int rga_probe(struct platform_device *pdev)
 	rga->cmdbuf_virt = dma_alloc_attrs(rga->dev, RGA_CMDBUF_SIZE,
 					   &rga->cmdbuf_phy, GFP_KERNEL,
 					   DMA_ATTR_WRITE_COMBINE);
+	if (!rga->cmdbuf_virt) {
+		ret = -ENOMEM;
+		goto rel_vdev;
+	}
 
 	rga->src_mmu_pages =
 		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
+	if (!rga->src_mmu_pages) {
+		ret = -ENOMEM;
+		goto free_dma;
+	}
 	rga->dst_mmu_pages =
 		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
+	if (rga->dst_mmu_pages) {
+		ret = -ENOMEM;
+		goto free_src_pages;
+	}
 
 	def_frame.stride = (def_frame.width * def_frame.fmt->depth) >> 3;
 	def_frame.size = def_frame.stride * def_frame.height;
@@ -894,7 +906,7 @@ static int rga_probe(struct platform_device *pdev)
 	ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1);
 	if (ret) {
 		v4l2_err(&rga->v4l2_dev, "Failed to register video device\n");
-		goto rel_vdev;
+		goto free_dst_pages;
 	}
 
 	v4l2_info(&rga->v4l2_dev, "Registered %s as /dev/%s\n",
@@ -902,10 +914,15 @@ static int rga_probe(struct platform_device *pdev)
 
 	return 0;
 
+free_dst_pages:
+	free_pages((unsigned long)rga->dst_mmu_pages, 3);
+free_src_pages:
+	free_pages((unsigned long)rga->src_mmu_pages, 3);
+free_dma:
+	dma_free_attrs(rga->dev, RGA_CMDBUF_SIZE, rga->cmdbuf_virt,
+		       rga->cmdbuf_phy, DMA_ATTR_WRITE_COMBINE);
 rel_vdev:
 	video_device_release(vfd);
-unreg_video_dev:
-	video_unregister_device(rga->vfd);
 unreg_v4l2_dev:
 	v4l2_device_unregister(&rga->v4l2_dev);
 err_put_clk:
-- 
2.30.2