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@xxxxxxxxxxxxxxxxxxxx Reporter: bianpan2010@xxxxxxxxxx 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. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
