linux-kernel - drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels

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: <YMdPcWZi4x7vnCxI@google.com>
Date:   Mon, 14 Jun 2021 21:45:37 +0900
From:   Sergey Senozhatsky <senozhatsky@...omium.org>
To:     Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Chris Wilson <chris@...is-wilson.co.uk>
Cc:     intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: drm/i915: __GFP_RETRY_MAYFAIL allocations in stable kernels

Hi,

We are observing some user-space crashes (sigabort, segfaults etc.)
under moderate memory pressure (pretty far from severe pressure) which
have one thing in common - restrictive GFP mask in setup_scratch_page().

For instance, (stable 4.19) drivers/gpu/drm/i915/i915_gem_gtt.c

(trimmed down version)

static int gen8_init_scratch(struct i915_address_space *vm)
{
        setup_scratch_page(vm, __GFP_HIGHMEM);

        vm->scratch_pt = alloc_pt(vm);
        vm->scratch_pd = alloc_pd(vm);
        if (use_4lvl(vm)) {
                vm->scratch_pdp = alloc_pdp(vm);
        }
}

gen8_init_scratch() function puts a rather inconsistent restrictions on mm.

Looking at it line by line:

setup_scratch_page() uses very restrictive gfp mask:
	__GFP_HIGHMEM | __GFP_ZERO | __GFP_RETRY_MAYFAIL

it doesn't try to reclaim anything and fails almost immediately.

alloc_pt() - uses more permissive gfp mask:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pd() - likewise:
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN

alloc_pdp() - very permissive gfp mask:
	GFP_KERNEL


So can all allocations in gen8_init_scratch() use
	GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN
?

E.g.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index a12430187108..e862680b9c93 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -792,7 +792,7 @@ alloc_pdp(struct i915_address_space *vm)
 
        GEM_BUG_ON(!use_4lvl(vm));
 
-       pdp = kzalloc(sizeof(*pdp), GFP_KERNEL);
+       pdp = kzalloc(sizeof(*pdp), I915_GFP_ALLOW_FAIL);
        if (!pdp)
                return ERR_PTR(-ENOMEM);
 
@@ -1262,7 +1262,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
 {
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1972,7 +1972,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_hw_ppgtt *ppgtt)
        u32 pde;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -3078,7 +3078,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---



It's quite similar on stable 5.4 - setup_scratch_page() uses restrictive
gfp mask again.

---
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f614646ed3f9..99d78b1052df 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -1378,7 +1378,7 @@ static int gen8_init_scratch(struct i915_address_space *vm)
                return 0;
        }
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -1753,7 +1753,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
        struct i915_page_directory * const pd = ppgtt->base.pd;
        int ret;
 
-       ret = setup_scratch_page(vm, __GFP_HIGHMEM);
+       ret = setup_scratch_page(vm, GFP_KERNEL | __GFP_HIGHMEM);
        if (ret)
                return ret;
 
@@ -2860,7 +2860,7 @@ static int ggtt_probe_common(struct i915_ggtt *ggtt, u64 size)
                return -ENOMEM;
        }
 
-       ret = setup_scratch_page(&ggtt->vm, GFP_DMA32);
+       ret = setup_scratch_page(&ggtt->vm, GFP_KERNEL | GFP_DMA32);
        if (ret) {
                DRM_ERROR("Scratch setup failed\n");
                /* iounmap will also get called at remove, but meh */
---