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 BD5E0C5479D for ; Mon, 9 Jan 2023 08:25:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236501AbjAIIYw (ORCPT ); Mon, 9 Jan 2023 03:24:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39030 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236534AbjAIIYF (ORCPT ); Mon, 9 Jan 2023 03:24:05 -0500 Received: from relay08.th.seeweb.it (relay08.th.seeweb.it [5.144.164.169]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 859516557 for ; Mon, 9 Jan 2023 00:24:03 -0800 (PST) Received: from SoMainline.org (unknown [89.205.227.209]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by m-r2.th.seeweb.it (Postfix) with ESMTPSA id BA0103EE8C; Mon, 9 Jan 2023 09:23:59 +0100 (CET) Date: Mon, 9 Jan 2023 09:23:57 +0100 From: Marijn Suijten To: Dmitry Baryshkov Cc: phone-devel@vger.kernel.org, Rob Clark , Abhinav Kumar , Vinod Koul , ~postmarketos/upstreaming@lists.sr.ht, AngeloGioacchino Del Regno , Konrad Dybcio , Martin Botka , Jami Kettunen , Sean Paul , David Airlie , Daniel Vetter , Stephen Boyd , Bjorn Andersson , Jessica Zhang , Ville =?utf-8?B?U3lyasOkbMOk?= , Kuogee Hsieh , Jani Nikula , sunliming , Sam Ravnborg , Haowen Bai , Konrad Dybcio , Loic Poulain , Vinod Polimera , Douglas Anderson , Vladimir Lypak , linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-kernel@vger.kernel.org, Drew Davenport Subject: Re: [PATCH v2 4/8] drm/msm/dpu: Disallow unallocated resources to be returned Message-ID: <20230109082357.meebk7udokdfvwle@SoMainline.org> References: <20221221231943.1961117-1-marijn.suijten@somainline.org> <20221221231943.1961117-5-marijn.suijten@somainline.org> <1b872a47-6ffc-1fe9-f283-897dbc37d709@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1b872a47-6ffc-1fe9-f283-897dbc37d709@linaro.org> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023-01-09 01:30:29, Dmitry Baryshkov wrote: > On 09/01/2023 01:28, Dmitry Baryshkov wrote: > > On 22/12/2022 01:19, Marijn Suijten wrote: > >> In the event that the topology requests resources that have not been > >> created by the system (because they are typically not represented in > >> dpu_mdss_cfg ^1), the resource(s) in global_state (in this case DSC > >> blocks) remain NULL but will still be returned out of > >> dpu_rm_get_assigned_resources, where the caller expects to get an array > >> containing num_blks valid pointers (but instead gets these NULLs). > >> > >> To prevent this from happening, where null-pointer dereferences > >> typically result in a hard-to-debug platform lockup, num_blks shouldn't > >> increase past NULL blocks and will print an error and break instead. > >> After all, max_blks represents the static size of the maximum number of > >> blocks whereas the actual amount varies per platform. > >> > >> ^1: which can happen after a git rebase ended up moving additions to > >> _dpu_cfg to a different struct which has the same patch context. > >> > >> Fixes: bb00a452d6f7 ("drm/msm/dpu: Refactor resource manager") > >> Signed-off-by: Marijn Suijten > >> --- > >>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 5 +++++ > >>   1 file changed, 5 insertions(+) > > > > I think the patch is not fully correct. Please check resource > > availability during allocation. I wouldn't expect an error from > > get_assigned_resources because of resource exhaustion. Theoretically patch 5/8 should take care of this, and we should never reach this failure condition. Emphasis on /should/, this may happen again if/when another block type is added with sub-par resource allocation and assignment implementation. > Another option, since allocation functions (except DSC) already have > these safety checks: check error message to mention internal > inconstency: allocated resource doesn't exist. Is this a suggestion for the wording of the error message? - Marijn