From nobody Thu Sep 19 01:45:29 2024 Received: from Edge12.fintech.ru (exchange.fintech.ru [195.54.195.159]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9446C54656 for ; Tue, 6 Aug 2024 17:19:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=195.54.195.159 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722964764; cv=none; b=afq+ZhazqYaKcsF2UbYc+qlWLflvZY5YdI6S6JaPvN+OLyM8Qo5613EnLcTH5DUaCK446MYBnvpwmw44QZeNA9s/IbYIvr3m3pKXFP3O4oA4NTzUhrcLnoCEpleRbQKvzy4/zyo6lxFRPNuC45lUjNCIDtVkXYMCKfDXgt2ApYg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1722964764; c=relaxed/simple; bh=5QAei2OX+YEU4UhkwTmkY8l4+ZE2Ab2sPTA1DDaPebM=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=H0KHV9HQhvqRGx9Eonfolqg1kCyPmHUT5iwW8XoNspE3NeX6YxZyQuDA1kI1rvazSHYKOgHZEEKXoxvg5CHMySiGxqRRbKS2Hkrg5rYFRmpvYFGPwU4qyOedPai1Du8GgZxvAlL99GIjnutUCNDY7rBuEHccxn1fHOiKnZO1y6k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=fintech.ru; spf=pass smtp.mailfrom=fintech.ru; arc=none smtp.client-ip=195.54.195.159 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=fintech.ru Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fintech.ru Received: from Ex16-01.fintech.ru (10.0.10.18) by exchange.fintech.ru (195.54.195.169) with Microsoft SMTP Server (TLS) id 14.3.498.0; Tue, 6 Aug 2024 20:19:11 +0300 Received: from localhost (10.0.253.138) by Ex16-01.fintech.ru (10.0.10.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.4; Tue, 6 Aug 2024 20:19:10 +0300 From: Nikita Zhandarovich To: Alex Deucher , =?UTF-8?q?Christian=20K=C3=B6nig?= , Xinhui Pan , David Airlie , Daniel Vetter CC: Nikita Zhandarovich , Jerome Glisse , Dave Airlie , , , , Subject: [PATCH v2] drm/radeon/evergreen_cs: fix int overflow errors in cs track offsets Date: Tue, 6 Aug 2024 10:19:04 -0700 Message-ID: <20240806171904.49032-1-n.zhandarovich@fintech.ru> X-Mailer: git-send-email 2.25.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: Ex16-02.fintech.ru (10.0.10.19) To Ex16-01.fintech.ru (10.0.10.18) Content-Type: text/plain; charset="utf-8" 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. Same goes for a few instances of 'surf.layer_size * mslice' multiplications that are added to 'offset' variable - they may potentially overflow as well and need to be validated properly. While some debug prints in this code section take possible overflow issues into account, simply casting to (unsigned long) may be erroneous in its own way, as depending on CPU architecture one is liable to get different results. Fix said problems by: - casting 'offset' to fixed u64 data type instead of ambiguous unsigned long. - casting one of the operands in vulnerable to integer overflow cases to u64. - adjust format specifiers in debug prints to properly represent 'offset' values. Found by Linux Verification Center (linuxtesting.org) with static analysis tool SVACE. Fixes: 285484e2d55e ("drm/radeon: add support for evergreen/ni tiling infor= mations v11") Signed-off-by: Nikita Zhandarovich --- v2: - change data type to cast from unsigned long to u64 per Alex's and Christian's suggestion: https://lore.kernel.org/all/CADnq5_NaMr+vpqwqhsMoSeGrto2Lw5v0KXWEp2HRK=3D++= orScMg@mail.gmail.com/ - include validation of surf.layer_size * mslice per Christian's approval: https://lore.kernel.org/all/1914cfcb-9700-4274-8120-9746e241cb54@amd.com/ - change format specifiers when printing 'offset' value. - fix commit description to reflect patch changes. v1: https://lore.kernel.org/all/20240725180950.15820-1-n.zhandarovich@fintech.r= u/ drivers/gpu/drm/radeon/evergreen_cs.c | 62 +++++++++++++++++--------------= ---- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen_cs.c b/drivers/gpu/drm/radeon= /evergreen_cs.c index e5577d2a19ef..a46613283393 100644 --- a/drivers/gpu/drm/radeon/evergreen_cs.c +++ b/drivers/gpu/drm/radeon/evergreen_cs.c @@ -397,7 +397,7 @@ static int evergreen_cs_track_validate_cb(struct radeon= _cs_parser *p, unsigned i struct evergreen_cs_track *track =3D p->track; struct eg_surface surf; unsigned pitch, slice, mslice; - unsigned long offset; + u64 offset; int r; =20 mslice =3D G_028C6C_SLICE_MAX(track->cb_color_view[id]) + 1; @@ -435,14 +435,14 @@ static int evergreen_cs_track_validate_cb(struct rade= on_cs_parser *p, unsigned i return r; } =20 - offset =3D track->cb_color_bo_offset[id] << 8; + offset =3D (u64)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", + dev_warn(p->dev, "%s:%d cb[%d] bo base %llu not aligned with %ld\n", __func__, __LINE__, id, offset, surf.base_align); return -EINVAL; } =20 - offset +=3D surf.layer_size * mslice; + offset +=3D (u64)surf.layer_size * mslice; if (offset > radeon_bo_size(track->cb_color_bo[id])) { /* old ddx are broken they allocate bo with w*h*bpp but * program slice with ALIGN(h, 8), catch this and patch @@ -450,14 +450,14 @@ static int evergreen_cs_track_validate_cb(struct rade= on_cs_parser *p, unsigned i */ if (!surf.mode) { uint32_t *ib =3D p->ib.ptr; - unsigned long tmp, nby, bsize, size, min =3D 0; + u64 tmp, nby, bsize, size, min =3D 0; =20 /* find the height the ddx wants */ if (surf.nby > 8) { min =3D surf.nby - 8; } bsize =3D radeon_bo_size(track->cb_color_bo[id]); - tmp =3D track->cb_color_bo_offset[id] << 8; + tmp =3D (u64)track->cb_color_bo_offset[id] << 8; for (nby =3D surf.nby; nby > min; nby--) { size =3D nby * surf.nbx * surf.bpe * surf.nsamples; if ((tmp + size * mslice) <=3D bsize) { @@ -469,7 +469,7 @@ static int evergreen_cs_track_validate_cb(struct radeon= _cs_parser *p, unsigned i slice =3D ((nby * surf.nbx) / 64) - 1; if (!evergreen_surface_check(p, &surf, "cb")) { /* check if this one works */ - tmp +=3D surf.layer_size * mslice; + tmp +=3D (u64)surf.layer_size * mslice; if (tmp <=3D bsize) { ib[track->cb_color_slice_idx[id]] =3D slice; goto old_ddx_ok; @@ -478,9 +478,9 @@ 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 %llu, max layer %d, bo size %ld, slice %d)\n", __func__, __LINE__, id, surf.layer_size, - track->cb_color_bo_offset[id] << 8, mslice, + (u64)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, @@ -564,7 +564,7 @@ static int evergreen_cs_track_validate_stencil(struct r= adeon_cs_parser *p) struct evergreen_cs_track *track =3D p->track; struct eg_surface surf; unsigned pitch, slice, mslice; - unsigned long offset; + u64 offset; int r; =20 mslice =3D G_028008_SLICE_MAX(track->db_depth_view) + 1; @@ -610,18 +610,18 @@ static int evergreen_cs_track_validate_stencil(struct= radeon_cs_parser *p) return r; } =20 - offset =3D track->db_s_read_offset << 8; + offset =3D (u64)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", + dev_warn(p->dev, "%s:%d stencil read bo base %llu not aligned with %ld\n= ", __func__, __LINE__, offset, surf.base_align); return -EINVAL; } - offset +=3D surf.layer_size * mslice; + offset +=3D (u64)surf.layer_size * mslice; if (offset > radeon_bo_size(track->db_s_read_bo)) { dev_warn(p->dev, "%s:%d stencil read bo too small (layer size %d, " - "offset %ld, max layer %d, bo size %ld)\n", + "offset %llu, max layer %d, bo size %ld)\n", __func__, __LINE__, surf.layer_size, - (unsigned long)track->db_s_read_offset << 8, mslice, + (u64)track->db_s_read_offset << 8, mslice, radeon_bo_size(track->db_s_read_bo)); dev_warn(p->dev, "%s:%d stencil invalid (0x%08x 0x%08x 0x%08x 0x%08x)\n", __func__, __LINE__, track->db_depth_size, @@ -629,18 +629,18 @@ static int evergreen_cs_track_validate_stencil(struct= radeon_cs_parser *p) return -EINVAL; } =20 - offset =3D track->db_s_write_offset << 8; + offset =3D (u64)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= ", + dev_warn(p->dev, "%s:%d stencil write bo base %llu not aligned with %ld\= n", __func__, __LINE__, offset, surf.base_align); return -EINVAL; } - offset +=3D surf.layer_size * mslice; + offset +=3D (u64)surf.layer_size * mslice; if (offset > radeon_bo_size(track->db_s_write_bo)) { dev_warn(p->dev, "%s:%d stencil write bo too small (layer size %d, " - "offset %ld, max layer %d, bo size %ld)\n", + "offset %llu, max layer %d, bo size %ld)\n", __func__, __LINE__, surf.layer_size, - (unsigned long)track->db_s_write_offset << 8, mslice, + (u64)track->db_s_write_offset << 8, mslice, radeon_bo_size(track->db_s_write_bo)); return -EINVAL; } @@ -661,7 +661,7 @@ static int evergreen_cs_track_validate_depth(struct rad= eon_cs_parser *p) struct evergreen_cs_track *track =3D p->track; struct eg_surface surf; unsigned pitch, slice, mslice; - unsigned long offset; + u64 offset; int r; =20 mslice =3D G_028008_SLICE_MAX(track->db_depth_view) + 1; @@ -708,34 +708,34 @@ static int evergreen_cs_track_validate_depth(struct r= adeon_cs_parser *p) return r; } =20 - offset =3D track->db_z_read_offset << 8; + offset =3D (u64)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", + dev_warn(p->dev, "%s:%d stencil read bo base %llu not aligned with %ld\n= ", __func__, __LINE__, offset, surf.base_align); return -EINVAL; } - offset +=3D surf.layer_size * mslice; + offset +=3D (u64)surf.layer_size * mslice; if (offset > radeon_bo_size(track->db_z_read_bo)) { dev_warn(p->dev, "%s:%d depth read bo too small (layer size %d, " - "offset %ld, max layer %d, bo size %ld)\n", + "offset %llu, max layer %d, bo size %ld)\n", __func__, __LINE__, surf.layer_size, - (unsigned long)track->db_z_read_offset << 8, mslice, + (u64)track->db_z_read_offset << 8, mslice, radeon_bo_size(track->db_z_read_bo)); return -EINVAL; } =20 - offset =3D track->db_z_write_offset << 8; + offset =3D (u64)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= ", + dev_warn(p->dev, "%s:%d stencil write bo base %llu not aligned with %ld\= n", __func__, __LINE__, offset, surf.base_align); return -EINVAL; } - offset +=3D surf.layer_size * mslice; + offset +=3D (u64)surf.layer_size * mslice; if (offset > radeon_bo_size(track->db_z_write_bo)) { dev_warn(p->dev, "%s:%d depth write bo too small (layer size %d, " - "offset %ld, max layer %d, bo size %ld)\n", + "offset %llu, max layer %d, bo size %ld)\n", __func__, __LINE__, surf.layer_size, - (unsigned long)track->db_z_write_offset << 8, mslice, + (u64)track->db_z_write_offset << 8, mslice, radeon_bo_size(track->db_z_write_bo)); return -EINVAL; }