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  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 22 Oct 2018 17:51:41 +0000
From:   "Kuehling, Felix" <>
To:     Wenwen Wang <>
CC:     "Zhou, David(ChunMing)" <>,
        David Airlie <>, Kangjie Lu <>,
        open list <>,
        "open list:RADEON and AMDGPU DRM DRIVERS" 
        "open list:DRM DRIVERS" <>,
        "Deucher, Alexander" <>,
        "Wang, Ken" <>,
        "Koenig, Christian" <>
Subject: Re: [PATCH] drm/amdgpu: fix a missing-check bug

The BIOS signature check does not guarantee integrity of the BIOS image
either way. As I understand it, the signature is just a magic number.
It's not a cryptographic signature. The check is just a sanity check.
Therefore this change doesn't add any meaningful protection against the
scenario you described.


On 2018-10-20 4:57 p.m., Wenwen Wang wrote:
> In amdgpu_read_bios_from_rom(), the header of the VBIOS is firstly copied
> to 'header' from an IO memory region through
> amdgpu_asic_read_bios_from_rom(). Then the header is checked to see whether
> it is a valid header. If yes, the whole VBIOS, including the header, is
> then copied to 'adev->bios'. The problem here is that no check is enforced
> on the header after the second copy. Given that the device also has the
> permission to access the IO memory region, it is possible for a malicious
> device controlled by an attacker to modify the header between these two
> copies. By doing so, the attacker can supply compromised VBIOS, which can
> cause undefined behavior of the kernel and introduce potential security
> issues.
> This patch rewrites the header in 'adev->bios' using the header acquired in
> the first copy.
> Signed-off-by: Wenwen Wang <>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 2 ++
>  1 file changed, 2 insertions(+)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index a5df80d..ac701f4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -181,6 +181,8 @@ static bool amdgpu_read_bios_from_rom(struct amdgpu_device *adev)
>  	/* read complete BIOS */
>  	amdgpu_asic_read_bios_from_rom(adev, adev->bios, len);
> +	memcpy(adev->bios, header, AMD_VBIOS_SIGNATURE_END);
> +
>  	if (!check_atom_bios(adev->bios, len)) {
>  		kfree(adev->bios);
>  		return false;

Powered by blists - more mailing lists