On Mon, 12 Jul 2021 at 16:17, Colin Ian King colin.king@canonical.com wrote:
Hi,
Static analysis with Coverity on linux-next has found a potential issue in drivers/gpu/drm/i915/selftests/intel_memory_region.c in function igt_mock_fill - the problematic commit is as follows:
commit d148738923fdb5077089e48ec15555e6008100d0 Author: Thomas Hellström thomas.hellstrom@linux.intel.com Date: Wed Jun 2 10:38:08 2021 +0200
drm/i915/ttm Initialize the ttm device and memory managersThe analysis is as follows:
49 static int igt_mock_fill(void *arg) 50 { 51 struct intel_memory_region *mem = arg; 52 resource_size_t total = resource_size(&mem->region); 53 resource_size_t page_size; 54 resource_size_t rem; 55 unsigned long max_pages; 56 unsigned long page_num; 57 LIST_HEAD(objects); 58 int err = 0; 59 60 page_size = mem->chunk_size; 61 rem = total; 62 retry:
value_overwrite: Overwriting previous write to max_pages with value from div64_u64(rem, page_size).
63 max_pages = div64_u64(rem, page_size); 64 65 for_each_prime_number_from(page_num, 1, max_pages) { 66 resource_size_t size = page_num * page_size; 67 struct drm_i915_gem_object *obj; 68 69 obj = i915_gem_object_create_region(mem, size, 0); 70 if (IS_ERR(obj)) { 71 err = PTR_ERR(obj); 72 break; 73 } 74 75 err = i915_gem_object_pin_pages_unlocked(obj); 76 if (err) { 77 i915_gem_object_put(obj); 78 break; 79 } 80 81 list_add(&obj->st_link, &objects); 82 rem -= size; 83 } 84 85 if (err == -ENOMEM) 86 err = 0; 87 if (err == -ENXIO) { 88 if (page_num * page_size <= rem) { 89 if (mem->is_range_manager && max_pages > 1) {
Unused value (UNUSED_VALUE) assigned_value: Assigning value from max_pages >> 1 to max_pages here, but that stored value is overwritten before it can be used.
Yeah, that doesn't look good.
AFAIK this should be fixed with d53ec322dc7d ("drm/i915/ttm: switch over to ttm_buddy_man"), which is in drm-tip, but I guess has not made its way over to linux-next yet.
Thanks for the report.
90 max_pages >>= 1; 91 goto retry; 92 } 93
The right shift to max_pages on line 90 is being overwritten on line 64 on the retry.
Colin