Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <546750a0-2619-5df2-482d-a6c22e209702@amd.com>
Date:   Fri, 11 Nov 2022 10:05:10 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.1
Subject: Re: [PATCH] amdgpu_dm_2027: Add pointer check
Content-Language: en-US
To:     Denis Arefev <arefev@swemel.ru>
Cc:     Leo Li <sunpeng.li@amd.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        trufanov@swemel.ru
References: <20221111071446.22951-1-arefev@swemel.ru>
From:   Harry Wentland <harry.wentland@amd.com>
In-Reply-To: <20221111071446.22951-1-arefev@swemel.ru>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dkRKbjZzbWx6UklkdmNZS2lqVS9nK3pRRUpxSTRQdlRuVy9OWmN0eWFpRjFp?=
 =?utf-8?B?SUF2ZXJDT1BjMm9ycEdaUHcrbGRPcW4rZlZLVld3ZXBQbmh3OU5OOGE1NmF5?=
 =?utf-8?B?c0hzR0RMbnd1TUY0Skh1VUE4TEV1QzFsSlRDdWUwQXNDSUgvYUs4M3VRTVVK?=
 =?utf-8?B?aEdnMVpJZi82NFluQ09QRklrVWhaVENlZktNWktqbDZ6K3dkN0MyTUNzb08z?=
 =?utf-8?B?cGU4NlV0WG1CdHM3T0IvTEpsKzU3TEFFWU5oT0RBYm9odEJCOUJWK2d6S1B4?=
 =?utf-8?B?dStVWGwrWHMyK3NuZ2kwaW5BYWZBOStNVnEvWi9zQWhhSEFqcTNRRlJ1bDd2?=
 =?utf-8?B?cWwzL2hWYmZHU1FGM25KWnZhS2FiNTBRMVF6UUJnK09MQTF3REYxVVFRWVBI?=
 =?utf-8?B?Z3dlajA0Um9BVVp0VjVlOHdydCt5WkpZZE43Zy9uUklSejltMlQvbCt3YW5l?=
 =?utf-8?B?cFowL3pQaGFEMHVFTkM2dDBXaVUxY0ROMUtxTGdTRWkyKzMzOCtTaS9RMy9v?=
 =?utf-8?B?ZFVTb3psZThNVmtrbGhXeXJ6YkxOa1h4bG4vV1FucWJhdXpRUzNyNXJ5c1M5?=
 =?utf-8?B?UVFSSWgzT2cvQk1DQlNtdy9XVEN0R0RMSEY5SkhLZUkrMlJtYzlmbExPaTBL?=
 =?utf-8?B?WS8xK1RCNGMrRmRLQjJUL3NEclhpcGN4OEh1cUcvLytkbVRlRmRZem1BOVk2?=
 =?utf-8?B?Y1o0UGlkdTAyaWMrYkR0akhEbVlFc1MyQmhScGpkdVE5YXE2clBGZEVDQk5n?=
 =?utf-8?B?by9QWmp4RkZvVExDVGxMdTROYm1LaExnVWJvRFhWS3JMeWZYMU8wUE00WG51?=
 =?utf-8?B?bHRjZEw4RGtSb3UxTFgrOXFtSUNCekQ4NG93dUc3bStPa09jR0hGWittWG1u?=
 =?utf-8?B?dzFOQkd0aTA4TVZMZENhZlQrSHpoRVVVNklSVk1wcktLTG41UTQ4enQ0SUU1?=
 =?utf-8?B?bFBsc0IreW0wWHYzZWY1S3lsTzdJRDU0d1o4ZTNRYjVFK3U1TlIyUEk1RUxK?=
 =?utf-8?B?cTROd0hEaHlaYmNNd1JnSVgvOXprKzF2YTc3TS9BZUFEbFZIa2Fyckhvb1Ev?=
 =?utf-8?B?RUF5VHpkbHlodGZoRERvT3QxYUk0dytyZ1V3TGp4UVVtU25oazEzWkJFK2c4?=
 =?utf-8?B?YytSY2RWRlhreHJlaHJOVFJTanVTMXJqcXVZMVJjMVF5MFFGL0tjcUZWeUQv?=
 =?utf-8?B?cW1HbTh2aEJwM3Ntb2loZkpOT281RVYrTEgwalJXU1ZlT2d4cWVNMWNvMUkz?=
 =?utf-8?B?blR4cEJmVW9ONXdDUWp2Vi9Lc0xVcmtQWWtYbE5yRGl5TldxWGZ6WTIvQXUw?=
 =?utf-8?B?bHFyOGJRMmRaYWhjc0V0NEZXZDU0cVpqUkxnUUVzMTR2MW5Bd0ZVcUdYODBC?=
 =?utf-8?B?Y0d2SitabEpKSWVtWkJxSW1ya2RTb003WSt6TGlqdWhlUitQUnFqbjFCQ0RP?=
 =?utf-8?B?blR4dFlCc0tiNDNEZXc3QUxjT25QanAzWk9meEFPOU12dmQ1azBWTWFrMjlZ?=
 =?utf-8?B?ZDNkUGdiamJUK0tNN3F1M25NUjUyUXJaVG1EVTUxcDZBZ3JTQVRLQTZmMHNM?=
 =?utf-8?B?S254SGwrOVUzRXNOVS9YSFpYTVVGdGMrYkFnQUgwS0hqVkNHN0V5bEI1bE54?=
 =?utf-8?B?MFN0eU9hYkZJbkZka2RCcTZYRERPQkloWStjVC95dlNONnY1QUduTmJYWFI4?=
 =?utf-8?B?SElVcmcxdlptd0ZrcWNlN0NnOXR1SGs5ekY2ME5NUGE2Wk9wTGZJeHRBdjlz?=
 =?utf-8?B?c0xUcUFCTmhhZldydU9ZZ2swcDV5MUxvaUlySk0wK3Jic2E5Z3BFdWZaQ3Zy?=
 =?utf-8?B?SDh6a3h1VVZmSk4zNU0xNEtMR2F5ZFI1ZmtmUEZsamVPMHF3Sm1kcUFUVmxn?=
 =?utf-8?B?cW45Tmx5Tk5hVmZFOUhjcXFYTmkySVVvMGQ0VURBMjFQK0hJZjU1ZnRHMGto?=
 =?utf-8?B?Z0ZkQXFDeWVnNHhPcDhTVTVRMGJkbEhCdy9UZnpUaHNtQndkMVAvVnNSeEdG?=
 =?utf-8?B?WEJYd3c1bEhIdTBNZUdVc1JoYXdDaS9QNjZ0bHM2bHM0OENXNnRQSzZLTkww?=
 =?utf-8?B?TUgwa1JNc0Vic0xVb21CZlZuQURRbDI4dlRFUWFzM29EblAvWTVCcjVyZlEw?=
 =?utf-8?Q?G9b0k3er/32a3p83W/BOWyiI+?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 68c37f04-a00b-4d2c-e823-08dac3f6230f
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5427.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Nov 2022 15:05:14.6475
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Iv3mIjY77RDCl9e4HO5fwqfnavMQ6iY443TbpsjeQ+XDU9/3fA5VJLxFOvGalpHXDydG+RPYWmj/DuE5zDlrgw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7631
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        SPF_HELO_PASS,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

The commit title should user the drm/amd/display prefix.
It might also be good to be more specific, like:
drm/amd/display: Check dc_resource_state_construct succeeded

On 11/11/22 02:14, Denis Arefev wrote:
> Return value of a function 'dc_create_state' is
> dereferenced at amdgpu_dm.c:2027 without checking for null
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 

Does this fix a test case? If so, please describe.

> Signed-off-by: Denis Arefev <arefev@swemel.ru>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 0f7749e9424d..529483997154 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1960,7 +1960,9 @@ static int dm_resume(void *handle)
>  	dc_release_state(dm_state->context);
>  	dm_state->context = dc_create_state(dm->dc);
>  	/* TODO: Remove dc_state->dccg, use dc->dccg directly. */
> -	dc_resource_state_construct(dm->dc, dm_state->context);
> +	if (dm_state->context) {
> +		dc_resource_state_construct(dm->dc, dm_state->context);
> +	}

I don't see how this won't leave the driver state in a
mess. Are you sure this isn't a fatal error?

Harry

>  
>  	/* Before powering on DC we need to re-initialize DMUB. */
>  	r = dm_dmub_hw_init(adev);