[PATCH v2 3/4] drm/exynos: added userptr feature.

[Jerome Glisse]

On Tue, May 8, 2012 at 3:59 AM, Inki Dae <inki.dae at samsung.com> wrote:
> Hi Jerome,
>
>> -----Original Message-----
>> From: Jerome Glisse [mailto:j.glisse at gmail.com]
>> Sent: Tuesday, May 08, 2012 3:18 AM
>> To: Dave Airlie
>> Cc: daeinki at gmail.com; Inki Dae; kyungmin.park at samsung.com;
>> sw0312.kim at samsung.com; dri-devel at lists.freedesktop.org
>> Subject: Re: [PATCH v2 3/4] drm/exynos: added userptr feature.
>>
>> On Sat, May 5, 2012 at 6:22 AM, Dave Airlie <airlied at gmail.com> wrote:
>> > On Sat, May 5, 2012 at 11:19 AM,  <daeinki at gmail.com> wrote:
>> >> Hi Dave,
>> >>
>> >> 2012. 4. 25. 오후 7:15 Dave Airlie <airlied at gmail.com> 작성:
>> >>
>> >>> On Tue, Apr 24, 2012 at 6:17 AM, Inki Dae <inki.dae at samsung.com>
> wrote:
>> >>>> this feature could be used to use memory region allocated by malloc()
>> in user
>> >>>> mode and mmaped memory region allocated by other memory allocators.
>> userptr
>> >>>> interface can identify memory type through vm_flags value and would
>> get
>> >>>> pages or page frame numbers to user space appropriately.
>> >>>
>> >>> Is there anything to stop the unpriviledged userspace driver locking
>> >>> all the RAM in the machine inside userptr?
>> >>>
>> >>
>> >> you mean that there is something that it can stop user space driver
>> locking some memory region  of RAM? and if any user space driver locked
>> some region then anyone on user space can't access the region? could you
>> please tell me about your concerns in more detail so that we can solve the
>> issue? I guess you mean that any user level driver such as specific EGL
>> library can allocate some memory region and also lock the region so that
>> other user space applications can't access the region until rendering is
>> completed by hw accelerator such as 2d/3d core or opposite case.
>> >>
>> >> actually, this feature has already been used by v4l2 so I didn't try to
>> consider we could face with any problem with this and I've got a feeling
>> maybe there is something I missed so I'd be happy for you or anyone give
>> me any advices.
>> >
>> > Well v4l get to make their own bad design decisions.
>> >
>> > The problem is if an unprivledged users accessing the drm can lock all
>> > the pages it allocates into memory, by passing them to the kernel as
>> > userptrs., thus bypassing the swap and blocking all other users on the
>> > system.
>> >
>> > Dave.
>>
>> Beside that you are not locking the vma and afaik this means that the
>> page backing the vma might change, yes you will still own the page you
>> get but userspace might be reading/writing to different pages. The vma
>
> Yes, right. the vma should be locked because the pages backing the vma
> might be changed after swap-in. thank you for your pointing.
>
>
>> would need to be locked but than the userspace might unlock it in your
>> back and you start right from the begining.
>>
>
> I'm not sure I understood your comments but my understanding is you mean
> that there is some interface that it can unlock(using some interface of drm
> side or implementing it?) the vma so we can resolve the issue Dave pointed
> out?
>
> Thanks,
> Inki Dae
>

Userspace can unlock the vma behind your back with the sys call
munlock, malicious process might use that to trick your gpu somehow.
The thing is there is no proper infrastructure in linux kernel for
locking vma for long period of time. Because it mess with core linux
vm stuff i believe the vm people should be involved and comment on
this. Also we absolutely don't want process to abuse the drm to force
locking, so you will need to make sure that what drm does won't go
over the memlock limit of the process (and i am not even sure that by
default this limit is not 0).

Anyway my point is i believe there is maybe no proper right now to do
this, maybe using VM_RESERVED but i am not clear on the implication of
doing so. Note in all case you will need the fragment the vma so that
you don't suddenly convert more than you want from anonymous vma to
something more restrictive.

Cheers,
Jerome