[PATCH 1/6] drm/i915: avoid "nearly-NULL" pointers
Dave Gordon
david.s.gordon at intel.com
Wed Jul 20 11:10:53 UTC 2016
Existing code sometimes calls to_intel_bo() with an unchecked pointer
that may be NULL, which in turn means that the possibly-NULL pointer
is passed to container_of(), potentially resulting in undefined or at
least unexpected behaviour. For example, the result may appear to be
an ERR_PTR() with an "impossible" value!
Correspondingly, we have numerous occurences of the strange construct
if (&obj->base == NULL) ...
which may technically yield undefined behaviour when obj == NULL. It is
in any case counterintuitive as to how any &-expression can be NULL.
These two constructs may also cause problems for Clang. See
https://lists.freedesktop.org/archives/mesa-dev/2012-April/020514.html
It "happens to work" at present because "base" is (and must be) the
very first member of the i915 object structure; so let's document that
requirement with a comment and some compile-time checking, so that
no-one accidentally breaks it later; and let's make the conversion from
a DRM GEM object to the driver's derived i915_drm_gem_object type-safe
using an inline function rather than a macro.
Thus assured that to_intel_bo(ptr) == NULL iff ptr == NULL, we can then
eliminate the weird (&obj->base) test and just write "if (obj == NULL)".
Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 8 +++++++-
drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++----------
drivers/gpu/drm/i915/i915_gem_tiling.c | 4 ++--
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_overlay.c | 2 +-
5 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e163a94..dab0594 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2157,6 +2157,7 @@ struct drm_i915_gem_object_ops {
(0xff << (INTEL_FRONTBUFFER_BITS_PER_PIPE * (pipe)))
struct drm_i915_gem_object {
+ /* Must be first */
struct drm_gem_object base;
const struct drm_i915_gem_object_ops *ops;
@@ -2286,7 +2287,12 @@ struct drm_i915_gem_object {
} userptr;
};
};
-#define to_intel_bo(x) container_of(x, struct drm_i915_gem_object, base)
+static inline struct drm_i915_gem_object *
+to_intel_bo(struct drm_gem_object *gem_obj)
+{
+ BUILD_BUG_ON(offsetof(struct drm_i915_gem_object, base));
+ return container_of(gem_obj, struct drm_i915_gem_object, base);
+}
static inline bool
i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 37868cc..2574569 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -865,7 +865,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -1281,7 +1281,7 @@ int i915_gem_obj_prepare_shmem_read(struct drm_i915_gem_object *obj,
goto put_rpm;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -1498,7 +1498,7 @@ static struct intel_rps_client *to_rps_client(struct drm_file *file)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -1547,7 +1547,7 @@ static struct intel_rps_client *to_rps_client(struct drm_file *file)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -1969,7 +1969,7 @@ static void i915_gem_object_free_mmap_offset(struct drm_i915_gem_object *obj)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -2793,7 +2793,7 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, args->bo_handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
mutex_unlock(&dev->struct_mutex);
return -ENOENT;
}
@@ -3597,7 +3597,7 @@ int i915_gem_get_caching_ioctl(struct drm_device *dev, void *data,
struct drm_i915_gem_object *obj;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL)
+ if (obj == NULL)
return -ENOENT;
switch (obj->cache_level) {
@@ -3658,7 +3658,7 @@ int i915_gem_set_caching_ioctl(struct drm_device *dev, void *data,
goto rpm_put;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -4027,7 +4027,7 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
@@ -4092,7 +4092,7 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
return ret;
obj = to_intel_bo(drm_gem_object_lookup(file_priv, args->handle));
- if (&obj->base == NULL) {
+ if (obj == NULL) {
ret = -ENOENT;
goto unlock;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index 8030199..7c3b700 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -167,7 +167,7 @@
int ret = 0;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL)
+ if (obj == NULL)
return -ENOENT;
if (!i915_tiling_ok(dev,
@@ -298,7 +298,7 @@
struct drm_i915_gem_object *obj;
obj = to_intel_bo(drm_gem_object_lookup(file, args->handle));
- if (&obj->base == NULL)
+ if (obj == NULL)
return -ENOENT;
mutex_lock(&dev->struct_mutex);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 51fbca7..5a5ef22 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15091,7 +15091,7 @@ static int intel_framebuffer_init(struct drm_device *dev,
struct drm_mode_fb_cmd2 mode_cmd = *user_mode_cmd;
obj = to_intel_bo(drm_gem_object_lookup(filp, mode_cmd.handles[0]));
- if (&obj->base == NULL)
+ if (obj == NULL)
return ERR_PTR(-ENOENT);
fb = intel_framebuffer_create(dev, &mode_cmd, obj);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 3212d88..6146b79 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1124,7 +1124,7 @@ int intel_overlay_put_image_ioctl(struct drm_device *dev, void *data,
new_bo = to_intel_bo(drm_gem_object_lookup(file_priv,
put_image_rec->bo_handle));
- if (&new_bo->base == NULL) {
+ if (new_bo == NULL) {
ret = -ENOENT;
goto out_free;
}
--
1.9.1
More information about the Intel-gfx-trybot
mailing list