[RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC EXYNOS4210.

[thomas@shipmail.org (Thomas Hellstrom)]

On 08/26/2011 01:47 PM, Inki Dae wrote:
> This patch is a DRM Driver for Samsung SoC Exynos4210 and now enables only FIMD yet
> but we will add HDMI support also in the future.
>
> this patch is based on git repository below:
> git://git.kernel.org/pub/scm/linux/kernel/git/airlied/drm-2.6.git,
> branch name: drm-next
> commit-id: bcc65fd8e929a9d9d34d814d6efc1d2793546922
>
> you can refer to our working repository below:
> http://git.infradead.org/users/kmpark/linux-2.6-samsung
> branch name: samsung-drm
>
> We tried to re-use lowlevel codes of the FIMD driver(s3c-fb.c
> based on Linux framebuffer) but couldn't so because lowlevel codes
> of s3c-fb.c are included internally and so FIMD module of this driver has
> its own lowlevel codes.
>
> We used GEM framework for buffer management and DMA APIs(dma_alloc_*)
> for buffer allocation. by using DMA API, we could use CMA later.
>
> Refer to this link for CMA(Continuous Memory Allocator):
> http://lkml.org/lkml/2011/7/20/45
>
> this driver supports only physically continuous memory(non-iommu).
>
> Links to previous versions of the patchset:
> v1:<  https://lwn.net/Articles/454380/>
> v2:<  http://www.spinics.net/lists/kernel/msg1224275.html>
>
> Changelog v2:
> DRM: add DRM_IOCTL_SAMSUNG_GEM_MMAP ioctl command.
>
>       this feature maps user address space to physical memory region
>       once user application requests DRM_IOCTL_SAMSUNG_GEM_MMAP ioctl.
>
> DRM: code clean and add exception codes.
>
> Changelog v3:
> DRM: Support multiple irq.
>
>       FIMD and HDMI have their own irq handler but DRM Framework can regiter only one irq handler
>       this patch supports mutiple irq for Samsung SoC.
>
> DRM: Consider modularization.
>
>       each DRM, FIMD could be built as a module.
>
> DRM: Have indenpendent crtc object.
>
>       crtc isn't specific to SoC Platform so this patch gets a crtc to be used as common object.
>       created crtc could be attached to any encoder object.
>
> DRM: code clean and add exception codes.
>
> S
>    

...

> +static struct drm_ioctl_desc samsung_ioctls[] = {
> +	DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_CREATE, samsung_drm_gem_create_ioctl,
> +			DRM_UNLOCKED),
> +	DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_MAP_OFFSET,
> +			samsung_drm_gem_map_offset_ioctl, DRM_UNLOCKED),
> +	DRM_IOCTL_DEF_DRV(SAMSUNG_GEM_MMAP,
> +			samsung_drm_gem_mmap_ioctl, DRM_UNLOCKED),
> +};
>    

What about security here? It looks to me like *any* user-space process 
can create a gem object and quickly exhaust available DMA memory space, 
potentially bringing the system down?

Likewise, there seems to be no owner check in the SAMSUNG_GEM_MMAP 
ioctl, allowing any user-space process unlimited graphics buffer access?

/Thomas