[Bug 188911] New: Function qxl_release_alloc() returns an improper value when the call to kmalloc() fails, resulting in bad memory access

[bugzilla-daemon at bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=188911

            Bug ID: 188911
           Summary: Function qxl_release_alloc() returns an improper value
                    when the call to kmalloc() fails, resulting in bad
                    memory access
           Product: Drivers
           Version: 2.5
    Kernel Version: linux-4.9-rc6
          Hardware: All
                OS: Linux
              Tree: Mainline
            Status: NEW
          Severity: normal
          Priority: P1
         Component: Video(DRI - non Intel)
          Assignee: drivers_video-dri at kernel-bugs.osdl.org
          Reporter: bianpan2010 at ruc.edu.cn
        Regression: No

(1) Function kmalloc() return a NULL pointer if there is no enough memory. The
function qxl_release_alloc() defined in file drivers/gpu/drm/qxl/qxl_release.c
tries to allocate memory and stores in its third parameter @@ret. Parameter
@@ret keeps unmodified if the call to kmalloc() (at line 133) fails. In this
case, it returns 0.
(2) Function qxl_alloc_release_reserved() calls qxl_release_alloc() to allocate
memory for its parameter @@release. By reviewing the source code of the callers
of function qxl_alloc_release_reserved() (e.g. qxl_process_single_command()
defined in file drivers/gpu/drm/qxl/qxl_ioctl.c), we find that parameter
@@release is uninitialized. The return value of qxl_release_alloc() is checked,
if the return value is 0, the execution flow will continue, and the memory
pointed by @@release will be accessed (at line 368). Recall that function
qxl_release_alloc() returns 0 when kmalloc() fails. In this case, the
uninitialized memory will be accessed, causing bad memory access.
(3) To avoid bad memory access, it's better to return "-ENOMEM" when the call
to kmalloc() fails in function qxl_release_alloc().

Codes related to this bug are summarised as follows.
(1) qxl_release_alloc @@ drivers/gpu/drm/qxl/qxl_release.c
125 static int
126 qxl_release_alloc(struct qxl_device *qdev, int type,
127           struct qxl_release **ret)
128 {
129     struct qxl_release *release;
130     int handle;
131     size_t size = sizeof(*release);
132 
133     release = kmalloc(size, GFP_KERNEL);
134     if (!release) {
135         DRM_ERROR("Out of memory\n");
136         return 0;  // "return -ENOMEM;"?
137     }
        ...
155     *ret = release;
156     QXL_INFO(qdev, "allocated release %d\n", handle);
157     release->id = handle;
158     return handle;
159 }

(2) qxl_alloc_release_reserved @@ drivers/gpu/drm/qxl/qxl_release.c
323 int qxl_alloc_release_reserved(struct qxl_device *qdev, unsigned long size,
324                        int type, struct qxl_release **release,
325                        struct qxl_bo **rbo)
326 {
327     struct qxl_bo *bo;
328     int idr_ret;
        ...
344     idr_ret = qxl_release_alloc(qdev, type, release);
345     if (idr_ret < 0) {
346         if (rbo)
347             *rbo = NULL;
348         return idr_ret;
349     }
        ...
366     bo = qxl_bo_ref(qdev->current_release_bo[cur_idx]);
367 
        // bad memory access when kmalloc() fails?
368     (*release)->release_offset = qdev->current_release_bo_offset[cur_idx] *
release_size_per_bo[cur_idx];
369     qdev->current_release_bo_offset[cur_idx]++;
        ...
388 }

(3) qxl_process_single_command @@ drivers/gpu/drm/qxl/qxl_ioctl.c
138 static int qxl_process_single_command(struct qxl_device *qdev,
139                                       struct drm_qxl_command *cmd,
140                                       struct drm_file *file_priv)
141 {
142         struct qxl_reloc_info *reloc_info;
143         int release_type;
144         struct qxl_release *release; // release is not initialized
            ...
175         ret = qxl_alloc_release_reserved(qdev,
176                                          sizeof(union qxl_release_info) +
177                                          cmd->command_size,
178                                          release_type,
179                                          &release,
180                                          &cmd_bo);
181         if (ret)
182                 goto out_free_reloc;
            ...
269 out_free_reloc:
270         kfree(reloc_info);
271         return ret;
272 }

Thanks very much!

-- 
You are receiving this mail because:
You are watching the assignee of the bug.