| 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
| ||
|
Message-ID: <1da9294d-3775-4387-be5f-408592c89bb7@amd.com>
Date: Fri, 26 Jul 2024 09:06:17 +0200
From: Christian König <christian.koenig@....com>
To: Alex Deucher <alexdeucher@...il.com>,
Nikita Zhandarovich <n.zhandarovich@...tech.ru>
Cc: Alex Deucher <alexander.deucher@....com>, Xinhui Pan
<Xinhui.Pan@....com>, David Airlie <airlied@...il.com>,
Daniel Vetter <daniel@...ll.ch>, Jerome Glisse <jglisse@...hat.com>,
Dave Airlie <airlied@...hat.com>, amd-gfx@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
lvc-project@...uxtesting.org, stable@...r.kernel.org
Subject: Re: [PATCH] drm/radeon/evergreen_cs: fix int overflow errors in cs
track offsets
I strongly suggest to revert that again. See my other mail.
Christian.
Am 25.07.24 um 22:59 schrieb Alex Deucher:
> Applied. Thanks!
>
> Alex
>
> On Thu, Jul 25, 2024 at 2:20 PM Nikita Zhandarovich
> <n.zhandarovich@...tech.ru> wrote:
>> Several cs track offsets (such as 'track->db_s_read_offset')
>> either are initialized with or plainly take big enough values that,
>> once shifted 8 bits left, may be hit with integer overflow if the
>> resulting values end up going over u32 limit.
>>
>> Some debug prints take this into account (see according dev_warn() in
>> evergreen_cs_track_validate_stencil()), even if the actual
>> calculated value assigned to local 'offset' variable is missing
>> similar proper expansion.
>>
>> Mitigate the problem by casting the type of right operands to the
>> wider type of corresponding left ones in all such cases.
>>
>> Found by Linux Verification Center (linuxtesting.org) with static
>> analysis tool SVACE.
>>
>> Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni tiling informations v11")
>> Cc: stable@...r.kernel.org
>> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
>> ---
>> P.S. While I am not certain that track->cb_color_bo_offset[id]
>> actually ends up taking values high enough to cause an overflow,
>> nonetheless I thought it prudent to cast it to ulong as well.
>>
>> drivers/gpu/drm/radeon/evergreen_cs.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon/evergreen_cs.c
>> index 1fe6e0d883c7..d734d221e2da 100644
>> --- a/drivers/gpu/drm/radeon/evergreen_cs.c
>> +++ b/drivers/gpu/drm/radeon/evergreen_cs.c
>> @@ -433,7 +433,7 @@ static int evergreen_cs_track_validate_cb(struct radeon_cs_parser *p, unsigned i
>> return r;
>> }
>>
>> - offset = track->cb_color_bo_offset[id] << 8;
>> + offset = (unsigned long)track->cb_color_bo_offset[id] << 8;
>> if (offset & (surf.base_align - 1)) {
>> dev_warn(p->dev, "%s:%d cb[%d] bo base %ld not aligned with %ld\n",
>> __func__, __LINE__, id, offset, surf.base_align);
>> @@ -455,7 +455,7 @@ static int evergreen_cs_track_validate_cb(struct radeon_cs_parser *p, unsigned i
>> min = surf.nby - 8;
>> }
>> bsize = radeon_bo_size(track->cb_color_bo[id]);
>> - tmp = track->cb_color_bo_offset[id] << 8;
>> + tmp = (unsigned long)track->cb_color_bo_offset[id] << 8;
>> for (nby = surf.nby; nby > min; nby--) {
>> size = nby * surf.nbx * surf.bpe * surf.nsamples;
>> if ((tmp + size * mslice) <= bsize) {
>> @@ -476,10 +476,10 @@ static int evergreen_cs_track_validate_cb(struct radeon_cs_parser *p, unsigned i
>> }
>> }
>> dev_warn(p->dev, "%s:%d cb[%d] bo too small (layer size %d, "
>> - "offset %d, max layer %d, bo size %ld, slice %d)\n",
>> + "offset %ld, max layer %d, bo size %ld, slice %d)\n",
>> __func__, __LINE__, id, surf.layer_size,
>> - track->cb_color_bo_offset[id] << 8, mslice,
>> - radeon_bo_size(track->cb_color_bo[id]), slice);
>> + (unsigned long)track->cb_color_bo_offset[id] << 8,
>> + mslice, radeon_bo_size(track->cb_color_bo[id]), slice);
>> dev_warn(p->dev, "%s:%d problematic surf: (%d %d) (%d %d %d %d %d %d %d)\n",
>> __func__, __LINE__, surf.nbx, surf.nby,
>> surf.mode, surf.bpe, surf.nsamples,
>> @@ -608,7 +608,7 @@ static int evergreen_cs_track_validate_stencil(struct radeon_cs_parser *p)
>> return r;
>> }
>>
>> - offset = track->db_s_read_offset << 8;
>> + offset = (unsigned long)track->db_s_read_offset << 8;
>> if (offset & (surf.base_align - 1)) {
>> dev_warn(p->dev, "%s:%d stencil read bo base %ld not aligned with %ld\n",
>> __func__, __LINE__, offset, surf.base_align);
>> @@ -627,7 +627,7 @@ static int evergreen_cs_track_validate_stencil(struct radeon_cs_parser *p)
>> return -EINVAL;
>> }
>>
>> - offset = track->db_s_write_offset << 8;
>> + offset = (unsigned long)track->db_s_write_offset << 8;
>> if (offset & (surf.base_align - 1)) {
>> dev_warn(p->dev, "%s:%d stencil write bo base %ld not aligned with %ld\n",
>> __func__, __LINE__, offset, surf.base_align);
>> @@ -706,7 +706,7 @@ static int evergreen_cs_track_validate_depth(struct radeon_cs_parser *p)
>> return r;
>> }
>>
>> - offset = track->db_z_read_offset << 8;
>> + offset = (unsigned long)track->db_z_read_offset << 8;
>> if (offset & (surf.base_align - 1)) {
>> dev_warn(p->dev, "%s:%d stencil read bo base %ld not aligned with %ld\n",
>> __func__, __LINE__, offset, surf.base_align);
>> @@ -722,7 +722,7 @@ static int evergreen_cs_track_validate_depth(struct radeon_cs_parser *p)
>> return -EINVAL;
>> }
>>
>> - offset = track->db_z_write_offset << 8;
>> + offset = (unsigned long)track->db_z_write_offset << 8;
>> if (offset & (surf.base_align - 1)) {
>> dev_warn(p->dev, "%s:%d stencil write bo base %ld not aligned with %ld\n",
>> __func__, __LINE__, offset, surf.base_align);
Powered by blists - more mailing lists