linux-kernel - re: drm/i915/ttm Initialize the ttm device and memory managers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <161dac7a-5aad-150e-7c14-7bb195ecf30f@canonical.com>
Date:   Mon, 12 Jul 2021 16:12:11 +0100
From:   Colin Ian King <colin.king@...onical.com>
To:     Thomas Hellström 
        <thomas.hellstrom@...ux.intel.com>,
        Matthew Auld <matthew.auld@...el.com>
Cc:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        intel-gfx <intel-gfx@...ts.freedesktop.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: re: drm/i915/ttm Initialize the ttm device and memory managers

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@...ux.intel.com>
Date:   Wed Jun 2 10:38:08 2021 +0200

    drm/i915/ttm Initialize the ttm device and memory managers

The 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.

 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