[drm/exynos] g2d: error handling and other things

[drm/exynos] g2d: error handling and other things

[Tobias Jakobi]

Again, sorry for the noise. These two patches should now really fix what
they're supposed to fix.

- Tobias

[PATCH 1/2] drm/exynos: g2d: make ioctls more robust

[Tobias Jakobi]

Both exynos_g2d_set_cmdlist_ioctl and exynos_g2d_exec_ioctl don't check
if the G2D was succesfully probe. If that is not the case, then g2d_priv
is just NULL and extracting 'dev' from it in the next step  is going to
produce a kernel oops.

Add proper checks and return ENODEV if the G2D is not available.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 5fa1bb6..8c62423 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1056,7 +1056,7 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 {
 	struct drm_exynos_file_private *file_priv = file->driver_priv;
 	struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
-	struct device *dev = g2d_priv->dev;
+	struct device *dev;
 	struct g2d_data *g2d;
 	struct drm_exynos_g2d_set_cmdlist *req = data;
 	struct drm_exynos_g2d_cmd *cmd;
@@ -1067,6 +1067,10 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
 	int size;
 	int ret;
 
+	if (!g2d_priv)
+		return -ENODEV;
+
+	dev = g2d_priv->dev;
 	if (!dev)
 		return -ENODEV;
 
@@ -1223,13 +1227,17 @@ int exynos_g2d_exec_ioctl(struct drm_device *drm_dev, void *data,
 {
 	struct drm_exynos_file_private *file_priv = file->driver_priv;
 	struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
-	struct device *dev = g2d_priv->dev;
+	struct device *dev;
 	struct g2d_data *g2d;
 	struct drm_exynos_g2d_exec *req = data;
 	struct g2d_runqueue_node *runqueue_node;
 	struct list_head *run_cmdlist;
 	struct list_head *event_list;
 
+	if (!g2d_priv)
+		return -ENODEV;
+
+	dev = g2d_priv->dev
 	if (!dev)
 		return -ENODEV;
 
-- 
1.8.5.5

Re: [PATCH 1/2] drm/exynos: g2d: make ioctls more robust

[Inki Dae]

2014-07-23 23:57 GMT+09:00 Tobias Jakobi <tjakobi@math.uni-bielefeld.de>:
> Both exynos_g2d_set_cmdlist_ioctl and exynos_g2d_exec_ioctl don't check
> if the G2D was succesfully probe. If that is not the case, then g2d_priv
> is just NULL and extracting 'dev' from it in the next step  is going to
> produce a kernel oops.
>
> Add proper checks and return ENODEV if the G2D is not available.
>
> Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
> ---
>  drivers/gpu/drm/exynos/exynos_drm_g2d.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> index 5fa1bb6..8c62423 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
> @@ -1056,7 +1056,7 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
>  {
>         struct drm_exynos_file_private *file_priv = file->driver_priv;
>         struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
> -       struct device *dev = g2d_priv->dev;
> +       struct device *dev;
>         struct g2d_data *g2d;
>         struct drm_exynos_g2d_set_cmdlist *req = data;
>         struct drm_exynos_g2d_cmd *cmd;
> @@ -1067,6 +1067,10 @@ int exynos_g2d_set_cmdlist_ioctl(struct drm_device *drm_dev, void *data,
>         int size;
>         int ret;
>
> +       if (!g2d_priv)
> +               return -ENODEV;
> +
> +       dev = g2d_priv->dev;
>         if (!dev)
>                 return -ENODEV;
>
> @@ -1223,13 +1227,17 @@ int exynos_g2d_exec_ioctl(struct drm_device *drm_dev, void *data,
>  {
>         struct drm_exynos_file_private *file_priv = file->driver_priv;
>         struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
> -       struct device *dev = g2d_priv->dev;
> +       struct device *dev;
>         struct g2d_data *g2d;
>         struct drm_exynos_g2d_exec *req = data;
>         struct g2d_runqueue_node *runqueue_node;
>         struct list_head *run_cmdlist;
>         struct list_head *event_list;
>
> +       if (!g2d_priv)
> +               return -ENODEV;
> +
> +       dev = g2d_priv->dev

It seems that you have no build test because above line incurs build
error. Anyway I fixed it up.

Thanks,
Inki Dae

>         if (!dev)
>                 return -ENODEV;
>
> --
> 1.8.5.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] drm/exynos: g2d: make ioctls more robust

[Tobias Jakobi]

Inki Dae wrote:
> It seems that you have no build test because above line incurs build
> error. Anyway I fixed it up.
That's why I have sent v2 of the patch.

With best wishes,
Tobias

Re: [PATCH 1/2] drm/exynos: g2d: make ioctls more robust

[Inki Dae]

On 2014년 08월 03일 18:52, Tobias Jakobi wrote:
> Inki Dae wrote:
>> It seems that you have no build test because above line incurs build
>> error. Anyway I fixed it up.
> That's why I have sent v2 of the patch.
> 

Oops, sorry. I didn't check v2.

Thanks,
Inki Dae


> With best wishes,
> Tobias
> 
> 

Re: [PATCH 1/2] drm/exynos: g2d: make ioctls more robust

[Tobias Jakobi]

On 2014-08-04 02:28, Inki Dae wrote:
> Oops, sorry. I didn't check v2.
No problem :)

It would be good if you could also check the libdrm patches that I've 
resent some while ago.

With best wishes,
Tobias

[PATCH 2/2] drm/exynos: g2d: let exynos_g2d_get_ver_ioctl fail

[Tobias Jakobi]

Currently the DRM_IOCTL_EXYNOS_G2D_GET_VER ioctl always succeeds, even
if no G2D support is available. Let the ioctl fail when this is the
case, so that userspace can accurately probe for G2D support.

This also fixes the exynos tests in libdrm. There 'g2d_init' doesn't
fail when G2D is absent, leading to a segfault later.

Signed-off-by: Tobias Jakobi <tjakobi@math.uni-bielefeld.de>
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 8c62423..d662ab6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -1042,8 +1042,23 @@ err:
 int exynos_g2d_get_ver_ioctl(struct drm_device *drm_dev, void *data,
 			     struct drm_file *file)
 {
+	struct drm_exynos_file_private *file_priv = file->driver_priv;
+	struct exynos_drm_g2d_private *g2d_priv = file_priv->g2d_priv;
+	struct device *dev;
+	struct g2d_data *g2d;
 	struct drm_exynos_g2d_get_ver *ver = data;
 
+	if (!g2d_priv)
+		return -ENODEV;
+
+	dev = g2d_priv->dev;
+	if (!dev)
+		return -ENODEV;
+
+	g2d = dev_get_drvdata(dev);
+	if (!g2d)
+		return -EFAULT;
+
 	ver->major = G2D_HW_MAJOR_VER;
 	ver->minor = G2D_HW_MINOR_VER;
 
-- 
1.8.5.5