linux-kernel - Re: [PATCH v3 8/9] drm/rockchip: gem: Don't alloc/free gem buf when dev

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKMK7uEjyBwJKmZUnXZLZ_YzgajFMKNem-2Eoc80ZDXha-7o6g@mail.gmail.com>
Date:   Fri, 7 Apr 2017 08:30:46 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     jeffy <jeffy.chen@...k-chips.com>
Cc:     Sean Paul <seanpaul@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Brian Norris <briannorris@...omium.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Tomasz Figa <tfiga@...omium.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        Chris Zhong <zyw@...k-chips.com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v3 8/9] drm/rockchip: gem: Don't alloc/free gem buf when
 dev_private is invalid

On Thu, Apr 6, 2017 at 1:09 PM, jeffy <jeffy.chen@...k-chips.com> wrote:
>
> On 04/06/2017 04:26 PM, Daniel Vetter wrote:
>>
>> On Wed, Apr 05, 2017 at 12:28:40PM -0400, Sean Paul wrote:
>>>
>>> On Wed, Apr 05, 2017 at 04:29:26PM +0800, Jeffy Chen wrote:
>>>>
>>>> After unbinding drm, the userspace may still has a chance to access
>>>> gem buf.
>>>>
>>>> Add a sanity check for a NULL dev_private to prevent that from
>>>> happening.
>>>
>>>
>>> I still don't understand how this is happening. You're saying that these
>>> hooks
>>> can be called after rockchip_drm_unbind() has finished?
>>
>>
>> Yeah this is supposed to be impossible. If it isn't, we need to debug and
>> fix this properly. This smells like pretty bad duct-tape ...
>
>
> it looks like after unbind, the user space may still own drm dev fd, and
> could be able to call ioctl:
> lrwx------. 1 chronos chronos 64 Mar 15 12:53 28 -> /dev/dri/card1 (deleted)
>
> and the drm_unplug_dev may help it, maybe we should call it in unbind? or
> just break drm_ioctl when drm_dev not registered?

Yes, by default unbind while userspace is running is totally broken in
drm. drm_unplug_dev would be the fix, but it's only used by udl and
not many use that. You might need to fix infrastructure up a bit.

For normal module unload the module reference will prevent unloading.
So why exactly do you care about the unbind use-case?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch