Date:   Thu, 25 Nov 2021 15:25:38 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Kyle Copperfield <kmcopper@danwin1210.me>
Cc:     Ezequiel Garcia <ezequiel@collabora.com>,
        Mauro Carvalho Chehab <mchehab@kernel.org>,
        Heiko Stuebner <heiko@sntech.de>,
        Dragan Simic <dragan.simic@gmail.com>,
        linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] media: rockchip/rga: do proper error checking in probe
Message-ID: <20211125122538.GF18178@kadam>
References: <20211120122321.20253-1-kmcopper@danwin1210.me>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211120122321.20253-1-kmcopper@danwin1210.me>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Ua6ndyV4eUEZCr12oA9jqbAOGl0g2SkcPCrOqhIrEWRPyzx2fD/tdvvGRCKp?=
 =?us-ascii?Q?smUNSZxPGLV6VPkgYD2o2L7XGy5kDJqS8ivYLYjHIKoYfxrx6Hu6QG6M0enO?=
 =?us-ascii?Q?wrxBUW+OColvupXaFc1xU+uW/VKk2sPdkLH5klNMvisvsqfh31xE9/gBORzb?=
 =?us-ascii?Q?UUPjc5wZ5XyYj+pMM0DUJX3TsRBHk8vCSSJkFkZ/fE8vSidldxTkPOFdXAjA?=
 =?us-ascii?Q?fkqPMmR3yj9I1XxkZ4FahagB7t0Sn/EyDM582k+4iuEqesQ4FiHQv/rF5RQH?=
 =?us-ascii?Q?r/RB3xhKvFGk36XF+xZuzgbJRrHTLAGGDPBWASCJ7Qp7fDB+WbpQpH4yoT/l?=
 =?us-ascii?Q?L+Kx7aEnGCySs9/lBzSb9+664zvqEPi3oz6rpMyIVuB2w+ilwXtUi+azaZO0?=
 =?us-ascii?Q?/VO9VyocP3DVgs/92ll8LaUtQrhJ+RHLvAuDHtO0t5ZLzzdlbswKsiky6Lug?=
 =?us-ascii?Q?WI79pmW2GWBYV3gBS25wQuMFiZcD33kmOjw2TJRGB5R8l3CgFfKnAK/0btIA?=
 =?us-ascii?Q?GkjBTHEfDWEOT5A8um+TyQxqbG065xwnKz3Bot9igktOsXnCmiPVXyMmX3uW?=
 =?us-ascii?Q?K4YCie4q5t/zxV1dQ5wbMyaZEEc6NGekieT6naqEyNTtz34dUgBNRCfTOEok?=
 =?us-ascii?Q?XeCub3hEno7jjq/Cas++2mqkHz7TNSqPiqRCPNp7xyf6PDIg0YzstFcqiAmC?=
 =?us-ascii?Q?tnNHg86yVQnR/HKmIDkxwoR5i9eGPtLHpDG7Vq0Tt5TAokDjGhFZJrxFsOrJ?=
 =?us-ascii?Q?gOAZ6m0bxv7+gIe2ffjEoX3pecOafcc2QP1LLpmnA0f7zMy99fYe0HaJra8f?=
 =?us-ascii?Q?zpqLB3NP9SCQbA6D9Q/OCeftHwgKJ1nXghn6laMa239ND8YtNb26GnF9Y2h6?=
 =?us-ascii?Q?MGGIsuEna9HQ6acQe49G/OiUrAy3mdk/849HFfWq88/dMcJRwtYmHJsuUcDB?=
 =?us-ascii?Q?7/tUfiq5lt7DOEJ5OnJN/JyuVMEHYhUNvjKK8p8eJMtk03coMWEYFy9z5m1n?=
 =?us-ascii?Q?uVuItDfsZdyqPibgBVaY7j4lSvmLl/UNSoJRIRVDyXBpcTvKfIhefTYhv6iw?=
 =?us-ascii?Q?S3LloKt2fLehFDb/vNjyQVSvVHjD95V0tBOu9aow5rI8dMmNH8Xn/DBMgCXE?=
 =?us-ascii?Q?AhhdOmKFBYGb0pUqjw13bY5NuYwi7pArkugYBHFVC3vMsPAQHV0fa7Jsk+n/?=
 =?us-ascii?Q?MNQMvkOKx8cuIDPhFSGY/6ubNjO6qD7ruLJXHqBbqt0bWTAtcMc/ctnPACmU?=
 =?us-ascii?Q?6n2zK39i3cgIHMuLdrj0VlKlOgFnX1slczFRaFMsgPB6sbDhgfcR74GbZGZh?=
 =?us-ascii?Q?kP2pCg5oOu0eYqZM+R7zEAUWoej/xqPLctFJWK9rMPKFBxpk5oqxa8HkxaKO?=
 =?us-ascii?Q?duLqwzLsXEzEaOKKJiSu/QsUglHhidFcfxD/woLb0V1NM6UxyRIZlKXjM6+D?=
 =?us-ascii?Q?uCAcrcyO/7IZXPCxDi8PCXONIFbnBz0JORPfwvjni1dcDmoGo78eV/lF2yhd?=
 =?us-ascii?Q?TBxy9udbyRZq8mHNBJK/21gyUc40NpdiKWL4uFwXJPrpQz7dk1eT+OP5Ct2g?=
 =?us-ascii?Q?l2jW/EJkuxNv9IfMHFsGIIq3ijfzA3RTcQ25ns7mSUsCOJBXUGrjQXsp/J4l?=
 =?us-ascii?Q?smmLlS5Eb64GUlEfGh4EcMNZuRA3oJachrpv4+8E28yvECIyGpKABrG4N5eM?=
 =?us-ascii?Q?l4X55gOe9x61HYkFP8lSDQ8l6hA=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 97f89e9b-fcc2-4e79-0d6b-08d9b00ebe26
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Nov 2021 12:26:02.0888
 (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: Om9HyNhwEuKaPgYpGo8txCB0mc+p/9QMudaFjy1bVXJCxWphB0hrX+sBZGvMSGwuYrx5Dlmz3JBMtz31LKu9S7zbGRXAh/D1RE6zyP0RgHE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB5861
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10178 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 spamscore=0
 mlxlogscore=999 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111250069
X-Proofpoint-GUID: ANASO9_yt9weuGCVHE_KZRDsSlZUMOIt
X-Proofpoint-ORIG-GUID: ANASO9_yt9weuGCVHE_KZRDsSlZUMOIt
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Nov 20, 2021 at 12:23:02PM +0000, Kyle Copperfield wrote:
> The latest fix for probe error handling contained a typo that causes
> probing to fail with the following message:
> 
>   rockchip-rga: probe of ff680000.rga failed with error -12
> 
> This patch fixes the typo.
> 
> Fixes: e58430e1d4fd (media: rockchip/rga: fix error handling in probe)
> Reviewed-by: Dragan Simic <dragan.simic@gmail.com>
> Signed-off-by: Kyle Copperfield <kmcopper@danwin1210.me>
> ---
>  drivers/media/platform/rockchip/rga/rga.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rockchip/rga/rga.c b/drivers/media/platform/rockchip/rga/rga.c
> index 6759091b15e0..d99ea8973b67 100644
> --- a/drivers/media/platform/rockchip/rga/rga.c
> +++ b/drivers/media/platform/rockchip/rga/rga.c
> @@ -895,7 +895,7 @@ static int rga_probe(struct platform_device *pdev)
>  	}
>  	rga->dst_mmu_pages =
>  		(unsigned int *)__get_free_pages(GFP_KERNEL | __GFP_ZERO, 3);
> -	if (rga->dst_mmu_pages) {
> +	if (!rga->dst_mmu_pages) {
>  		ret = -ENOMEM;
>  		goto free_src_pages;
>  	}

There was supposed to be Smatch warning to catch this bug but Smatch was
not parsing __get_free_pages() correctly.  I've fixed that so it
generates a warning now.

drivers/media/platform/rockchip/rga/rga.c:928 rga_probe() warn: 'rga->dst_mmu_pages' from __get_free_pages() not released on lines: 928.

Also I have introduced a new checker warning for code like:

	foo = alloc();
	if (foo)
		ret = -ENOMEM;

drivers/media/platform/rockchip/rga/rga.c:896 rga_probe() warn: inverted NULL check

I will test that out tonight.  I've also created a warning for if we
return success when a function returns NULL but I'm not really
optimistic that it will work.  Still, you never know until you check so
I'll test that out as well.

drivers/media/platform/rockchip/rga/rga.c:912 rga_probe() warn: success when 'rga->dst_mmu_pages' is NULL'

regards,
dan carpenter