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: <2bb95e80-b60a-36c0-76c8-a06833032c77@amd.com>
Date:   Tue, 5 Jul 2022 13:39:20 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v8 2/2] drm/gem: Don't map imported GEMs
Content-Language: en-US
To:     Dmitry Osipenko <dmitry.osipenko@collabora.com>,
        David Airlie <airlied@linux.ie>,
        Gerd Hoffmann <kraxel@redhat.com>,
        Gurchetan Singh <gurchetansingh@chromium.org>,
        Chia-I Wu <olvaffe@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <mripard@kernel.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Emil Velikov <emil.l.velikov@gmail.com>,
        =?UTF-8?Q?Thomas_Hellstr=c3=b6m?= <thomas_os@shipmail.org>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        Dmitry Osipenko <digetx@gmail.com>,
        linux-tegra@vger.kernel.org, kernel@collabora.com,
        virtualization@lists.linux-foundation.org
References: <20220701090240.1896131-1-dmitry.osipenko@collabora.com>
 <20220701090240.1896131-3-dmitry.osipenko@collabora.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20220701090240.1896131-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?L0lZMnBqZDRGMVhoN05qNUZNTy9YdCt1d2hpdUFBbHF2YllJa3YxWFJFYm1P?=
 =?utf-8?B?VmNVU2RTQW0zaTM2dTBzVW5ZKzIrdjRFdEZjeW5ReHE2dFU3MTROT0pVMklN?=
 =?utf-8?B?UW5McS9IMlBzWDV0bE8zanYvemROcUcrSTR1Vnh6TEdMQWMwOFZYNTM3T0ll?=
 =?utf-8?B?aXBjSlp6L3RNeGFyeGt2YTdhLzRMcnRZT2ZBa0FLZFFlUEk2aENhaFg4NmlX?=
 =?utf-8?B?c1lVcVJOdUo3MjhaWkl4RUV6dHRnZGdzWEJYUHkweXZUVDVvUGZtdjU0WVg4?=
 =?utf-8?B?MG5GOXJvQ3B3RUd2TGszb1YzeTJuSkltOGpPdS9ReXJta3R1L2xFLzlueE42?=
 =?utf-8?B?ckdyOUx2QVBJUStVUUxxd044MUxEcUdSK0pLVlRUdkdLUCs4VW9vN25PWnUv?=
 =?utf-8?B?VEVyTGZRQjlQL001djJCemdpQjMwR0krZCtwQ0VmQ1dwMVNmQk9IWUNvbTdE?=
 =?utf-8?B?Zzk4SFpMYkxyRUhjdTkwVFFmN2lJa25NL3VldEFNNXJicWZOK29WVnFnZ3NF?=
 =?utf-8?B?K21LcFozS2ROaGtJa3BUVG01d1QyRzdzdHZURVBNSlF0U2sxZmhxaW1zb3pJ?=
 =?utf-8?B?eTA2RTh2aTF6N0VWMFhkUVBrRW5WUmU4aDZicFhSUVlDdWVqSXVqNVJjS1p6?=
 =?utf-8?B?Q0NzbWdMcS9ha3ZJZkdmYXQ3SHBYSUlMNlc5NEJuVTNLQldQM0tNblE2Y0Vt?=
 =?utf-8?B?WW9QV2YwWlJSb2VJZWtiejJNdjZmU2F3M1pTOTBRcnNxL0NsclliSmh3TERZ?=
 =?utf-8?B?UmtLRHBsMk03R3JybUg1aWp1RDlWRGRMcW8zODJQa0tPVUFWNUx5RUpNYk9H?=
 =?utf-8?B?KzBDSlJxQkxvWEFSb3UxQTFpQTU1c0grRmVDWWFiaXNuRFZhdmU3TU1jMlY0?=
 =?utf-8?B?SjhkQUNvVGNFZmJXcmo1RzR0dkVqMFhKU2UybTkzTHdubjIzdjdOVkliQlZa?=
 =?utf-8?B?ZnN2dEYvMjhEZDI0TWxJN3NvU1ZJcnZieGlHNE9GVXVrRlBid2RJdVc0NWtj?=
 =?utf-8?B?RFlsRDlERG4wOHFwRlQrb0QxbktNcThoUm03enhJMDVMRFFseGFUSTZaUzlZ?=
 =?utf-8?B?bDB2UU9zYnAzcnV0U096UXhJS3NEV0tZejdTdzRQNS9idFlpUE4wYUJ5aS9B?=
 =?utf-8?B?cWpZZDdydTZQZ1VacWNtdUo4K0s4bGlJZ3ZmVkhmMjM0cFVuSkNiTzlTK2pr?=
 =?utf-8?B?ODZKWGVGN3ZKRGpJeFk5Y2oxL1RjTGdOTUhhLzhiWnhad0Z6cXZ4ZVI1dFZG?=
 =?utf-8?B?MkdCMm83MXhkY3BtVXN6bHBNRzcySTRrM1FpOFVQYTlqSTFZKzNhWUZDdGJ5?=
 =?utf-8?B?dGtrTGErbXNVeDhPT3M4eHBQL1R3OHEvRHkwdEN1TzhGaHJCTkNoM0tOVFc4?=
 =?utf-8?B?U0F2Qk00aWN2SnZXakJSV1ZJeURVdVBwTDNxRk9taXcydGtkZHBxZm0vdk5t?=
 =?utf-8?B?OERuQ2FPUk8zVDdpRlVQRXZMYiswSTA2bjl0QkR3MktQQ3NoM2JYTlNkQjAw?=
 =?utf-8?B?TzVtenJxQlVvVUtHaHFVL0xPZ2poYmRqM3JFTVhwUzNoemxJam55K3RQNm9s?=
 =?utf-8?B?VFdqejZsYnQxck9HZkNtd0w0c05OR1I4enpqZkZUYXdhUGdISXlRN1dTdzBU?=
 =?utf-8?B?OC9WMlZpcmFlUjhNVStkbUZHeVNIZ3lrU253Ym9jdkEzSEprV2R6R01kNCtl?=
 =?utf-8?B?SmdFdVhDOVROK3dTMXdTVkQ4TTliQUh5Nm1qbm5qZFBvOHA4NCtLSmZYYW1T?=
 =?utf-8?B?bEllSzNtQnJUcXYyOUd0dGdlODVDNWxTZStPZVc2SUhuSmYvcytKbWpBNUNC?=
 =?utf-8?B?SDk2QUpjZ2ZCNFpuaEJublQ2Y2RPQ2Y4VndacW12QUdZTHphd29seDdGSERs?=
 =?utf-8?B?WkFMZ09lRHFXTzVvUnFBVDE5TFVUTmh6c0pFend1dlJmSlIzOC9Sc05jb2V5?=
 =?utf-8?B?WFVrV0RpMVVIdHBVOTlWbTJpMVhmcXh5T0lmb1BkT0ZZUnNsMWJEV3dTSHFM?=
 =?utf-8?B?SDJxRldYOTF5V0FWWm5VMzFIL0NINWdFVHZDcWNFNVoveExlMXU3cHYyczhn?=
 =?utf-8?B?QTUxbWxYaGtadDc2ckljbUhjcTc4T2dDSGFCRVdZc01rZllvWmtBWVc1bE9R?=
 =?utf-8?Q?J3u/J4OOsNmhhd4mVxEdC4n5M?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 338052e8-da48-42ec-1e94-08da5e7b0493
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2022 11:39:27.9272
 (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: vhrDG4CkCs7BPCqORgpsPG3xyRFbBDynlhHUnJXvjofnxQa9vgRVq86jm44x9Jg5
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5134
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,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        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 01.07.22 um 11:02 schrieb Dmitry Osipenko:
> Drivers that use drm_gem_mmap() and drm_gem_mmap_obj() helpers don't
> handle imported dma-bufs properly, which results in mapping of something
> else than the imported dma-buf. On NVIDIA Tegra we get a hard lockup when
> userspace writes to the memory mapping of a dma-buf that was imported into
> Tegra's DRM GEM.
>
> Majority of DRM drivers prohibit mapping of the imported GEM objects.
> Mapping of imported GEMs require special care from userspace since it
> should sync dma-buf because mapping coherency of the exporter device may
> not match the DRM device. Let's prohibit the mapping for all DRM drivers
> for consistency.
>
> Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

I'm pretty sure that this is the right approach, but it's certainly more 
than possible that somebody abused this already.

Anyway patch is Reviewed-by: Christian König <christian.koenig@amd.com> 
since you are really fixing a major stability problem here.

Regards,
Christian.

> ---
>   drivers/gpu/drm/drm_gem.c              | 4 ++++
>   drivers/gpu/drm/drm_gem_shmem_helper.c | 9 ---------
>   2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 86d670c71286..fc9ec42fa0ab 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1034,6 +1034,10 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>   {
>   	int ret;
>   
> +	/* Don't allow imported objects to be mapped */
> +	if (obj->import_attach)
> +		return -EINVAL;
> +
>   	/* Check for valid size. */
>   	if (obj_size < vma->vm_end - vma->vm_start)
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 8ad0e02991ca..6190f5018986 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -609,17 +609,8 @@ EXPORT_SYMBOL_GPL(drm_gem_shmem_vm_ops);
>    */
>   int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma)
>   {
> -	struct drm_gem_object *obj = &shmem->base;
>   	int ret;
>   
> -	if (obj->import_attach) {
> -		/* Drop the reference drm_gem_mmap_obj() acquired.*/
> -		drm_gem_object_put(obj);
> -		vma->vm_private_data = NULL;
> -
> -		return dma_buf_mmap(obj->dma_buf, vma, 0);
> -	}
> -
>   	ret = drm_gem_shmem_get_pages(shmem);
>   	if (ret) {
>   		drm_gem_vm_close(vma);