linux-kernel - Re: [Linaro-mm-sig] Re: [PATCH v2] drm/gem: Fix GEM handle release errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <509d8bb0-f030-f8ca-a610-da5faaa6396b@amd.com>
Date:   Wed, 10 Aug 2022 11:16:04 +0200
From:   Christian König <christian.koenig@....com>
To:     Chen Jeffy <jeffy.chen@...k-chips.com>,
        Daniel Vetter <daniel.vetter@...ll.ch>
Cc:     Andy Yan <andy.yan@...k-chips.com>,
        Jianqun Xu <jay.xu@...k-chips.com>,
        Maxime Ripard <mripard@...nel.org>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linaro-mm-sig@...ts.linaro.org, David Airlie <airlied@...ux.ie>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        linux-media@...r.kernel.org, Daniel Vetter <daniel@...ll.ch>
Subject: Re: [Linaro-mm-sig] Re: [PATCH v2] drm/gem: Fix GEM handle release
 errors

Hi Jeffy,

Am 10.08.22 um 06:16 schrieb Chen Jeffy:
> Hi Christian,
>
> On 8/9 星期二 18:18, Christian König wrote:
>> Hi Jeffy,
>> [SNIP]
>>> Maybe cache the latest returned handle in the obj(after 
>>> drm_gem_prime_fd_to_handle), and clear it when that handle been 
>>> deleted in drm_gem_handle_delete()?
>>
>> That won't work. The handle is per fpriv, but the same object is used 
>> by multiple fpriv instances. >
>> What we could maybe do is to prevent adding multiple lockup 
>> structures when there is already one, but that's not something I can 
>> easily judge.
>
> So maybe we need to protect that unique lookup structure been deleted 
> before deleting the last handle, and make the handle unique for GEM 
> obj, in case of that unique lookup's handle been deleted earlier that 
> others?
>
> How about adding a GEM obj rbtree too, and make drm_prime_member kref-ed?
>
> So the 
> drm_prime_add_buf_handle/drm_gem_handle_create_tail/drm_gem_handle_delete 
> would be looking up drm_prime_member by GEM obj, then update dmabuf rb 
> and inc/dec drm_prime_member's kref, 
> drm_gem_prime_fd_to_handle/drm_gem_prime_handle_to_fd remain unchanged.

I think we should probably come up with an idea what the UAPI should 
look like before we try to implement this in the kernel, but in general 
I think we should make the solution simpler and not even more complex.

Recording multiple handles for the same DMA-buf/fpriv combination 
doesn't seem to make sense, so the duplicated tracking of 
handle->dma_buf mapping just seems to be overkill.

So my proposal would be this:
1. Only the first used GEM handle is tracker for each DMA-buf/fpriv 
combination.
2. Imports either return this first used handle or allocate a new one if 
there isn't any.
3. If the first used handle is closed we allocate a new one on re-import 
even when there duplicate handles.

The alternative as we have it now is to just return a more or less 
random handle if there are duplicates which doesn't sound like something 
we would want.

Daniel, can we agree on that?

Regards,
Christian.