linux-kernel - Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZVQ3d8FFqxsy0OX7@intel.com>
Date:   Tue, 14 Nov 2023 22:13:59 -0500
From:   Rodrigo Vivi <rodrigo.vivi@...el.com>
To:     Paz Zcharya <pazz@...omium.org>
CC:     <matthew.auld@...el.com>,
        Tvrtko Ursulin <tvrtko.ursulin@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Subrata Banik <subratabanik@...gle.com>,
        <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        Sean Paul <seanpaul@...omium.org>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        "Marcin Wojtas" <mwojtas@...omium.org>,
        Drew Davenport <ddavenport@...omium.org>,
        David Airlie <airlied@...il.com>,
        <intel-gfx@...ts.freedesktop.org>,
        "Nirmoy Das" <nirmoy.das@...el.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Fix phys_base to be
 relative not absolute

On Sun, Nov 05, 2023 at 05:27:03PM +0000, Paz Zcharya wrote:
> Fix the value of variable `phys_base` to be the relative offset in
> stolen memory, and not the absolute offset of the GSM.

to me it looks like the other way around. phys_base is the physical
base address for the frame_buffer. Setting it to zero doesn't seem
to make that relative. And also doesn't look right.

> 
> Currently, the value of `phys_base` is set to "Surface Base Address,"
> which in the case of Meter Lake is 0xfc00_0000.

I don't believe this is a fixed value. IIRC this comes from the register
set by video bios, where the idea is to reuse the fb that was used so
far.

With this in mind I don't understand how that could overflow. Maybe
the size of the stolen is not right? maybe the size? maybe different
memory region?

> This causes the
> function `i915_gem_object_create_region_at` to fail in line 128, when
> it attempts to verify that the range does not overflow:
> 
> if (range_overflows(offset, size, resource_size(&mem->region)))
>       return ERR_PTR(-EINVAL);
> 
> where:
>   offset = 0xfc000000
>   size = 0x8ca000
>   mem->region.end + 1 = 0x4400000
>   mem->region.start = 0x800000
>   resource_size(&mem->region) = 0x3c00000
> 
> call stack:
>   i915_gem_object_create_region_at
>   initial_plane_vma
>   intel_alloc_initial_plane_obj
>   intel_find_initial_plane_obj
>   intel_crtc_initial_plane_config
> 
> Looking at the flow coming next, we see that `phys_base` is only used
> once, in function `_i915_gem_object_stolen_init`, in the context of
> the offset *in* the stolen memory. Combining that with an
> examinination of the history of the file seems to indicate the
> current value set is invalid.
> 
> call stack (functions using `phys_base`)
>   _i915_gem_object_stolen_init
>   __i915_gem_object_create_region
>   i915_gem_object_create_region_at
>   initial_plane_vma
>   intel_alloc_initial_plane_obj
>   intel_find_initial_plane_obj
>   intel_crtc_initial_plane_config
> 
> [drm:_i915_gem_object_stolen_init] creating preallocated stolen
> object: stolen_offset=0x0000000000000000, size=0x00000000008ca000
> 
> Signed-off-by: Paz Zcharya <pazz@...omium.org>
> ---
> 
>  drivers/gpu/drm/i915/display/intel_plane_initial.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> index a55c09cbd0e4..e696cb13756a 100644
> --- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> +++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> @@ -90,7 +90,7 @@ initial_plane_vma(struct drm_i915_private *i915,
>  			"Using phys_base=%pa, based on initial plane programming\n",
>  			&phys_base);
>  	} else {
> -		phys_base = base;
> +		phys_base = 0;
>  		mem = i915->mm.stolen_region;
>  	}
>  
> -- 
> 2.42.0.869.gea05f2083d-goog
>