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: <68dd2e87-c4d6-99de-22bf-5059d03c22bb@amd.com>
Date:   Wed, 2 Nov 2022 10:15:56 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH v2 2/2] drm/client: Prevent NULL dereference in
 drm_client_buffer_delete()
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, noralf@tronnes.org,
        Dan Carpenter <dan.carpenter@oracle.com>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20221030154412.8320-1-dmitry.osipenko@collabora.com>
 <20221030154412.8320-3-dmitry.osipenko@collabora.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20221030154412.8320-3-dmitry.osipenko@collabora.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N2NGNUROUXdSSzhGc2xlRGZCMXhNN1Fudi83eHpTeHJuVTlhWkxnL2krQVEy?=
 =?utf-8?B?aldEdEtPeTJ3UzVTekNUOEFKbDBPclBGUjhVRi9JYlZjREdRR0ZQYWhPWm92?=
 =?utf-8?B?Y1plaUtNY05ZWnppZ0ZseUJIYmRIL1ZreDR2bGJWQVBEaGJUQXJmWi8xV24y?=
 =?utf-8?B?YmpJbFZTNjlJZTVicnlHNjRGN1RlQjZkaVdKVXQ2ZWl3WHVwWjkrZVpobG1T?=
 =?utf-8?B?bDlnREZib0JlMEZSeDdiTEFHWFk5ZmF2dkhMOEozRDNKV1NBZlV4dll0Y1M4?=
 =?utf-8?B?NGowMnRuVS9xSnBMRHhZOHpQb2g1M2RCUFdySVBzbWUzbGVPWkRVbnNjQ2dE?=
 =?utf-8?B?VDZydy9tbGRlbG42MkdCYWFFclBHNFBIdVR1SFJFZnJsR2s0VzQ1Y2s1RzQ2?=
 =?utf-8?B?SDlCRFg3RUpaZnBleEFHTTJrQTkyaEQxVDJOR2ZyV0J5L0NmbG02bjByNlFJ?=
 =?utf-8?B?elNKRWo1VDRTa2xXdlRnZUVqRmFzR1lyUzIwZ3VqWUtCUWhDbHFzTEFXa3F3?=
 =?utf-8?B?Q2Z1d2hCRjZScXVRTFdpZVlVWnhKcXNac0hzNVJWYjNNaWdTamttMHVWSGxp?=
 =?utf-8?B?d3MzTTRvZEdLTEVJRVNHWGovMnF5TTRYOU41eCtaTGJZQkNEeHhCS1dMU2NN?=
 =?utf-8?B?Q281WWlRNHBpdWl4d2JkcXBwMis1MlNaaWVURldvVCtibTNDQ3FySFBsQUcv?=
 =?utf-8?B?WEROWFBMRnBpK1hGaVJvUW92cFdQd0dJdHJ4UWVTUStCcTViekRYbThlMFE5?=
 =?utf-8?B?U1FmemY3NUlwZy9MbXV4SXcvVS9taUZickRrRVBxR0N3K3hVMm5vNlNNM2hU?=
 =?utf-8?B?UnpOaTdib0hjbzBmZzBSaFdleS9PQjZ5cTdzR3libWtkZFFhRXpZTEtDMHBT?=
 =?utf-8?B?eXBoaDdYcndsS3oxek0yQ0VsMCtQaUIvVC9DVzl3LzFhc2M2TThCRDcxQ2lZ?=
 =?utf-8?B?OHpiSzJSWVpaM3pzR3NMYSs5TFJtdEtpY1ZEeFVHSnVvT0xnaWFRN2N2THpI?=
 =?utf-8?B?UkVXM092cVY1ZXZmWWxDNjRKbEQ1MEhDRXNlcWRJdlBSVnRwUWR0NU5pa3B3?=
 =?utf-8?B?OU10R3VMNU1QQmFvRzhIcEdkTWowUzlOYWhLd0Q5UVdNWnVuOFRGU3hUUHlk?=
 =?utf-8?B?cjVabEFkK00rQ1piSEl3RENBbHJESE8vVks0czFmQllDOVJ2VGtBR2pERzBX?=
 =?utf-8?B?OFFVK3ZuU05YZU1UYlZxUW1GcWxPcFA4UGhEakQ4Tkpwa0kwQlAvN3BCNitX?=
 =?utf-8?B?TkV2SnFmZ3FkOEtLTnVpZU14MG50K09QYWVRQXYwMDFDNjJWbThWSUs2dDA5?=
 =?utf-8?B?Wm5XR3ovWWJiRk9obTBrMmN1bkVQaW9QdWFySDhVTWhYOEhnVGU3WVFOVVk2?=
 =?utf-8?B?Z3kzdEVxMWhQaEl2VTJ0MlBzQU0xZzVLc0xTWkZMVENHdTJlK3FOY25TdWh4?=
 =?utf-8?B?QkdWWXUvNjNOMXlsQmc3dUkzc1dNU2lwa2NJd2QrVlpxTE43Uy9YOVpZa25m?=
 =?utf-8?B?Z0JGVkZCemVuMVdjelVSNE1ndUdRVU8wUXBHZDJWeStEejMwMWJQbEh4elRB?=
 =?utf-8?B?VTJjcTlVRllzTFNabEo2TERqOVR2R3FZdGljRjBCa3ZVZ2QxbHZpQkJRdk51?=
 =?utf-8?B?a3RFVmh3MUo2MmpsbE5FejBHcytwSzlJQWh1Zy8rajU4NEhyMm1sa1o3Tkp6?=
 =?utf-8?B?d1hpYmgxY0xiM1NqK1pxT0dzUUw3ZE5FOWowNU9COU5WOHpGOWV0ZVRTUzZR?=
 =?utf-8?B?cXRlOURCZGI5bFUybGZ1VEk0Ykd6M1k3K094eEZud2ZLa1R3TDlZeWd4a0l0?=
 =?utf-8?B?R0E4cXpyVFNtRTBmeEVDVjRmY3pWZWFob2ZhdyszcGNyZUZxZ0R6cWZzNlha?=
 =?utf-8?B?MHJoZHk1U3ZWdkJyeXl4RVlwUVNDajFiRkdTR0tocms2Rm5IVEpFQkRuYTVu?=
 =?utf-8?B?N1JwQlFYYkw2bUNuS2g5dFpoTmRFd2sybDd3c2JNUy9lK3Z6a0VQOHoyajF1?=
 =?utf-8?B?b2VlSTB5UkVxWENRRlJYM3c1SlUrWVJSL1pyaEdsZE1vVi9RMVFwUG9ZclRL?=
 =?utf-8?B?QzFaRWhxMXg0TUl2dG9aVTFHbFJNck9vSTl1dFF5U3dkcktDRjZyZjk5Nnl6?=
 =?utf-8?B?VGRsL0JLWDRKRmpHcU11Zmt1U3pFdDlBU0xPSjlNM2xMTXBKUDYwUXl3eEs3?=
 =?utf-8?Q?5To/cfwzDJ3ePmpR/AzR3zpCfkmROlaL0MQy5/Skqvdx?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6178a1fa-0a82-44d3-ce54-08dabcb2dd08
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2022 09:16:02.7544
 (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: kj+QM3Bs/zLEaQ94ZCFnH/Q4lRVc2QaAmAcuP9yWHa1RVkB0GQR9qsZJpkP81wkS
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7777
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,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

Am 30.10.22 um 16:44 schrieb Dmitry Osipenko:
> The drm_gem_vunmap() will crash with a NULL dereference if the passed
> object pointer is NULL. It wasn't a problem before we added the locking
> support to drm_gem_vunmap function because the mapping argument was always
> NULL together with the object. Make drm_client_buffer_delete() to check
> whether GEM is NULL before trying to unmap the GEM, it will happen on
> framebuffer creation error.
>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Link: https://lore.kernel.org/dri-devel/Y1kFEGxT8MVlf32V@kili/
> Fixes: 79e2cf2e7a19 ("drm/gem: Take reservation lock for vmap/vunmap operations")
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/drm_client.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index 38e1be991caa..fd67efe37c63 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,10 +235,10 @@ static void drm_client_buffer_delete(struct drm_client_buffer *buffer)
>   {
>   	struct drm_device *dev = buffer->client->dev;
>   
> -	drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
> -
> -	if (buffer->gem)
> +	if (buffer->gem) {
> +		drm_gem_vunmap_unlocked(buffer->gem, &buffer->map);
>   		drm_gem_object_put(buffer->gem);
> +	}
>   
>   	if (buffer->handle)
>   		drm_mode_destroy_dumb(dev, buffer->handle, buffer->client->file);