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: <CADnq5_NuAL4=hMyc6G0QkbSrjCXa6qFM-bFtt3A7DY6cCmCt9w@mail.gmail.com>
Date: Thu, 25 Jul 2024 16:59:16 -0400
From: Alex Deucher <alexdeucher@...il.com>
To: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
Cc: Alex Deucher <alexander.deucher@....com>, 
	Christian König <christian.koenig@....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

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

Powered by Openwall GNU/*/Linux Powered by OpenVZ