After the recent addition of drm_malloc_gfp(), it was noticed that
some callers of these functions has swapped the parameters in the
call - it's supposed to be 'number of members' and 'sizeof(element)',
but a few callers had got the size first and the count second. This
isn't otherwise detected because they're both type 'size_t', and
the implementation at present just multiplies them anyway, so the
result is still right. But some future implementation might treat
them differently (e.g. allowing 0 elements but not zero size), so
let's add some compile-time checks and complain if the second (size)
parameter isn't a sizeof() expression, or at least a compile-time
constant.

This patch also fixes those callers where the order was wrong.

v6: removed duplicate BUILD_BUG_ON_MSG(); avoided renaming functions
    by shadowing them with #defines and then calling the function
    (non-recursively!) from inside the #define [Chris Wilson]

Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>Cc: dri-
Cc: dri-devel at lists.freedesktop.org
---
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  2 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c   |  8 ++++----
 include/drm/drm_mem_util.h                   | 27 ++++++++++++++++++++++++---
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c 
b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 1aba01a..9ae4a71 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -340,7 +340,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void 
*data,
         */
        bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
        relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
-       stream = drm_malloc_ab(1, args->stream_size);
+       stream = drm_malloc_ab(args->stream_size, sizeof(*stream));
        cmdbuf = etnaviv_gpu_cmdbuf_new(gpu, ALIGN(args->stream_size, 8) + 8,
                                        args->nr_bos);
        if (!bos || !relocs || !stream || !cmdbuf) {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f734b3c..1a136d9 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1686,8 +1686,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
        }

        /* Copy in the exec list from userland */
-       exec_list = drm_malloc_ab(sizeof(*exec_list), args->buffer_count);
-       exec2_list = drm_malloc_ab(sizeof(*exec2_list), args->buffer_count);
+       exec_list = drm_malloc_ab(args->buffer_count, sizeof(*exec_list));
+       exec2_list = drm_malloc_ab(args->buffer_count, sizeof(*exec2_list));
        if (exec_list == NULL || exec2_list == NULL) {
                DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
                          args->buffer_count);
@@ -1775,8 +1775,8 @@ static bool only_mappable_for_reloc(unsigned int flags)
                return -EINVAL;
        }

-       exec2_list = drm_malloc_gfp(sizeof(*exec2_list),
-                                   args->buffer_count,
+       exec2_list = drm_malloc_gfp(args->buffer_count,
+                                   sizeof(*exec2_list),
                                    GFP_TEMPORARY);
        if (exec2_list == NULL) {
                DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
diff --git a/include/drm/drm_mem_util.h b/include/drm/drm_mem_util.h
index 741ce75..5b0111c 100644
--- a/include/drm/drm_mem_util.h
+++ b/include/drm/drm_mem_util.h
@@ -29,7 +29,7 @@

 #include <linux/vmalloc.h>

-static __inline__ void *drm_calloc_large(size_t nmemb, size_t size)
+static __inline__ void *drm_calloc_large(size_t nmemb, const size_t size)
 {
        if (size != 0 && nmemb > SIZE_MAX / size)
                return NULL;
@@ -41,8 +41,15 @@ static __inline__ void *drm_calloc_large(size_t nmemb, 
size_t size)
                         GFP_KERNEL | __GFP_HIGHMEM | __GFP_ZERO, PAGE_KERNEL);
 }

+#define        drm_calloc_large(nmemb, size)                                   
\
+({                                                                     \
+       BUILD_BUG_ON_MSG(!__builtin_constant_p(size),                   \
+               "Non-constant 'size' - check argument ordering?");      \
+       (drm_calloc_large)(nmemb, size);                                \
+})
+
 /* Modeled after cairo's malloc_ab, it's like calloc but without the zeroing. 
*/
-static __inline__ void *drm_malloc_ab(size_t nmemb, size_t size)
+static __inline__ void *drm_malloc_ab(size_t nmemb, const size_t size)
 {
        if (size != 0 && nmemb > SIZE_MAX / size)
                return NULL;
@@ -54,7 +61,14 @@ static __inline__ void *drm_malloc_ab(size_t nmemb, size_t 
size)
                         GFP_KERNEL | __GFP_HIGHMEM, PAGE_KERNEL);
 }

-static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t size, gfp_t gfp)
+#define        drm_malloc_ab(nmemb, size)                                      
\
+({                                                                     \
+       BUILD_BUG_ON_MSG(!__builtin_constant_p(size),                   \
+               "Non-constant 'size' - check argument ordering?");      \
+       (drm_malloc_ab)(nmemb, size);                                   \
+})
+
+static __inline__ void *drm_malloc_gfp(size_t nmemb, const size_t size, gfp_t 
gfp)
 {
        if (size != 0 && nmemb > SIZE_MAX / size)
                return NULL;
@@ -73,6 +87,13 @@ static __inline__ void *drm_malloc_gfp(size_t nmemb, size_t 
size, gfp_t gfp)
                         gfp | __GFP_HIGHMEM, PAGE_KERNEL);
 }

+#define        drm_malloc_gfp(nmemb, size, gfp)                                
\
+({                                                                     \
+       BUILD_BUG_ON_MSG(!__builtin_constant_p(size),                   \
+               "Non-constant 'size' - check argument ordering?");      \
+       (drm_malloc_gfp)(nmemb, size, gfp);                             \
+})
+
 static __inline void drm_free_large(void *ptr)
 {
        kvfree(ptr);
-- 
1.9.1

Reply via email to