Skip to content

Commit 9cc0e5e

Browse files
ShumWengSangShumWengSang
authored
Merge pull request #733 from microsoft/fix/computepitch-64bit-overflow
Guard against 64-bit overflow in ComputePitch
2 parents 0bb96f0 + d8f2e2a commit 9cc0e5e

1 file changed

Lines changed: 93 additions & 38 deletions

File tree

‎DirectXTex/DirectXTexUtil.cpp‎

Lines changed: 93 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -953,6 +953,51 @@ size_t DirectX::BytesPerBlock(DXGI_FORMAT fmt) noexcept
953953
}
954954

955955

956+
namespace
957+
{
958+
// Multiplies two 64-bit values, setting 'overflow' if the mathematical result is
959+
// not representable. Returns 0 in that case; callers test the flag once after all
960+
// computations rather than at every individual site.
961+
inline uint64_t MulOverflow(uint64_t a, uint64_t b, bool& overflow) noexcept
962+
{
963+
#if defined(__GNUC__) || defined(__clang__)
964+
uint64_t result = 0;
965+
if (__builtin_mul_overflow(a, b, &result))
966+
{
967+
overflow = true;
968+
return 0;
969+
}
970+
return result;
971+
#elif defined(_MSC_VER) && (defined(_M_ARM64) || defined(_M_ARM64EC))
972+
const uint64_t high = __umulh(a, b);
973+
const uint64_t result = a * b;
974+
if (high != 0)
975+
{
976+
overflow = true;
977+
return 0;
978+
}
979+
return result;
980+
#elif defined(_MSC_VER) && defined(_M_X64)
981+
uint64_t high = 0;
982+
const uint64_t result = _umul128(a, b, &high);
983+
if (high != 0)
984+
{
985+
overflow = true;
986+
return 0;
987+
}
988+
return result;
989+
#else
990+
const uint64_t result = a * b;
991+
if ((a != 0) && ((result / a) != b))
992+
{
993+
overflow = true;
994+
return 0;
995+
}
996+
return result;
997+
#endif
998+
}
999+
}
1000+
9561001
//-------------------------------------------------------------------------------------
9571002
// Computes the image row pitch in bytes, and the slice ptich (size in bytes of the image)
9581003
// based on DXGI format, width, and height
@@ -963,6 +1008,7 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
9631008
{
9641009
uint64_t pitch = 0;
9651010
uint64_t slice = 0;
1011+
bool overflow = false;
9661012

9671013
switch (static_cast<int>(fmt))
9681014
{
@@ -981,15 +1027,15 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
9811027
{
9821028
const size_t nbw = width >> 2;
9831029
const size_t nbh = height >> 2;
984-
pitch = std::max<uint64_t>(1u, uint64_t(nbw) * 8u);
985-
slice = std::max<uint64_t>(1u, pitch * uint64_t(nbh));
1030+
pitch = std::max<uint64_t>(1u, MulOverflow(nbw, 8u, overflow));
1031+
slice = std::max<uint64_t>(1u, MulOverflow(pitch, nbh, overflow));
9861032
}
9871033
else
9881034
{
989-
const uint64_t nbw = std::max<uint64_t>(1u, (uint64_t(width) + 3u) / 4u);
990-
const uint64_t nbh = std::max<uint64_t>(1u, (uint64_t(height) + 3u) / 4u);
991-
pitch = nbw * 8u;
992-
slice = pitch * nbh;
1035+
const uint64_t nbw = std::max<uint64_t>(1u, (uint64_t(width) >> 2) + ((width & 3u) ? 1u : 0u));
1036+
const uint64_t nbh = std::max<uint64_t>(1u, (uint64_t(height) >> 2) + ((height & 3u) ? 1u : 0u));
1037+
pitch = MulOverflow(nbw, 8u, overflow);
1038+
slice = MulOverflow(pitch, nbh, overflow);
9931039
}
9941040
}
9951041
break;
@@ -1015,15 +1061,15 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
10151061
{
10161062
const size_t nbw = width >> 2;
10171063
const size_t nbh = height >> 2;
1018-
pitch = std::max<uint64_t>(1u, uint64_t(nbw) * 16u);
1019-
slice = std::max<uint64_t>(1u, pitch * uint64_t(nbh));
1064+
pitch = std::max<uint64_t>(1u, MulOverflow(nbw, 16u, overflow));
1065+
slice = std::max<uint64_t>(1u, MulOverflow(pitch, nbh, overflow));
10201066
}
10211067
else
10221068
{
1023-
const uint64_t nbw = std::max<uint64_t>(1u, (uint64_t(width) + 3u) / 4u);
1024-
const uint64_t nbh = std::max<uint64_t>(1u, (uint64_t(height) + 3u) / 4u);
1025-
pitch = nbw * 16u;
1026-
slice = pitch * nbh;
1069+
const uint64_t nbw = std::max<uint64_t>(1u, (uint64_t(width) >> 2) + ((width & 3u) ? 1u : 0u));
1070+
const uint64_t nbh = std::max<uint64_t>(1u, (uint64_t(height) >> 2) + ((height & 3u) ? 1u : 0u));
1071+
pitch = MulOverflow(nbw, 16u, overflow);
1072+
slice = MulOverflow(pitch, nbh, overflow);
10271073
}
10281074
}
10291075
break;
@@ -1032,15 +1078,15 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
10321078
case DXGI_FORMAT_G8R8_G8B8_UNORM:
10331079
case DXGI_FORMAT_YUY2:
10341080
assert(IsPacked(fmt));
1035-
pitch = ((uint64_t(width) + 1u) >> 1) * 4u;
1036-
slice = pitch * uint64_t(height);
1081+
pitch = MulOverflow((uint64_t(width) >> 1) + (width & 1u), 4u, overflow);
1082+
slice = MulOverflow(pitch, height, overflow);
10371083
break;
10381084

10391085
case DXGI_FORMAT_Y210:
10401086
case DXGI_FORMAT_Y216:
10411087
assert(IsPacked(fmt));
1042-
pitch = ((uint64_t(width) + 1u) >> 1) * 8u;
1043-
slice = pitch * uint64_t(height);
1088+
pitch = MulOverflow((uint64_t(width) >> 1) + (width & 1u), 8u, overflow);
1089+
slice = MulOverflow(pitch, height, overflow);
10441090
break;
10451091

10461092
case DXGI_FORMAT_NV12:
@@ -1051,8 +1097,8 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
10511097
return E_INVALIDARG;
10521098
}
10531099
assert(IsPlanar(fmt));
1054-
pitch = ((uint64_t(width) + 1u) >> 1) * 2u;
1055-
slice = pitch * (uint64_t(height) + ((uint64_t(height) + 1u) >> 1));
1100+
pitch = MulOverflow((uint64_t(width) >> 1) + (width & 1u), 2u, overflow);
1101+
slice = MulOverflow(pitch, uint64_t(height) + ((uint64_t(height) >> 1) + (height & 1u)), overflow);
10561102
break;
10571103

10581104
case DXGI_FORMAT_P010:
@@ -1075,20 +1121,20 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
10751121
case XBOX_DXGI_FORMAT_R16_UNORM_X8_TYPELESS:
10761122
case XBOX_DXGI_FORMAT_X16_TYPELESS_G8_UINT:
10771123
assert(IsPlanar(fmt));
1078-
pitch = ((uint64_t(width) + 1u) >> 1) * 4u;
1079-
slice = pitch * (uint64_t(height) + ((uint64_t(height) + 1u) >> 1));
1124+
pitch = MulOverflow((uint64_t(width) >> 1) + (width & 1u), 4u, overflow);
1125+
slice = MulOverflow(pitch, uint64_t(height) + ((uint64_t(height) >> 1) + (height & 1u)), overflow);
10801126
break;
10811127

10821128
case DXGI_FORMAT_NV11:
10831129
assert(IsPlanar(fmt));
1084-
pitch = ((uint64_t(width) + 3u) >> 2) * 4u;
1085-
slice = pitch * uint64_t(height) * 2u;
1130+
pitch = MulOverflow((uint64_t(width) >> 2) + ((width & 3u) ? 1u : 0u), 4u, overflow);
1131+
slice = MulOverflow(MulOverflow(pitch, height, overflow), 2u, overflow);
10861132
break;
10871133

10881134
case WIN10_DXGI_FORMAT_P208:
10891135
assert(IsPlanar(fmt));
1090-
pitch = ((uint64_t(width) + 1u) >> 1) * 2u;
1091-
slice = pitch * uint64_t(height) * 2u;
1136+
pitch = MulOverflow((uint64_t(width) >> 1) + (width & 1u), 2u, overflow);
1137+
slice = MulOverflow(MulOverflow(pitch, height, overflow), 2u, overflow);
10921138
break;
10931139

10941140
case WIN10_DXGI_FORMAT_V208:
@@ -1099,13 +1145,13 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
10991145
}
11001146
assert(IsPlanar(fmt));
11011147
pitch = uint64_t(width);
1102-
slice = pitch * (uint64_t(height) + (((uint64_t(height) + 1u) >> 1) * 2u));
1148+
slice = MulOverflow(pitch, uint64_t(height) + (((uint64_t(height) >> 1) + (height & 1u)) * 2u), overflow);
11031149
break;
11041150

11051151
case WIN10_DXGI_FORMAT_V408:
11061152
assert(IsPlanar(fmt));
11071153
pitch = uint64_t(width);
1108-
slice = pitch * (uint64_t(height) + (uint64_t(height >> 1) * 4u));
1154+
slice = MulOverflow(pitch, uint64_t(height) + (uint64_t(height >> 1) * 4u), overflow);
11091155
break;
11101156

11111157
default:
@@ -1129,42 +1175,51 @@ HRESULT DirectX::ComputePitch(DXGI_FORMAT fmt, size_t width, size_t height,
11291175
{
11301176
if (flags & CP_FLAGS_PAGE4K)
11311177
{
1132-
pitch = ((uint64_t(width) * bpp + 32767u) / 32768u) * 4096u;
1133-
slice = pitch * uint64_t(height);
1178+
pitch = MulOverflow((MulOverflow(width, bpp, overflow) + 32767u) / 32768u, 4096u, overflow);
1179+
slice = MulOverflow(pitch, height, overflow);
11341180
}
11351181
else if (flags & CP_FLAGS_ZMM)
11361182
{
1137-
pitch = ((uint64_t(width) * bpp + 511u) / 512u) * 64u;
1138-
slice = pitch * uint64_t(height);
1183+
pitch = MulOverflow((MulOverflow(width, bpp, overflow) + 511u) / 512u, 64u, overflow);
1184+
slice = MulOverflow(pitch, height, overflow);
11391185
}
11401186
else if (flags & CP_FLAGS_YMM)
11411187
{
1142-
pitch = ((uint64_t(width) * bpp + 255u) / 256u) * 32u;
1143-
slice = pitch * uint64_t(height);
1188+
pitch = MulOverflow((MulOverflow(width, bpp, overflow) + 255u) / 256u, 32u, overflow);
1189+
slice = MulOverflow(pitch, height, overflow);
11441190
}
11451191
else if (flags & CP_FLAGS_PARAGRAPH)
11461192
{
1147-
pitch = ((uint64_t(width) * bpp + 127u) / 128u) * 16u;
1148-
slice = pitch * uint64_t(height);
1193+
pitch = MulOverflow((MulOverflow(width, bpp, overflow) + 127u) / 128u, 16u, overflow);
1194+
slice = MulOverflow(pitch, height, overflow);
11491195
}
11501196
else // DWORD alignment
11511197
{
11521198
// Special computation for some incorrectly created DDS files based on
11531199
// legacy DirectDraw assumptions about pitch alignment
1154-
pitch = ((uint64_t(width) * bpp + 31u) / 32u) * sizeof(uint32_t);
1155-
slice = pitch * uint64_t(height);
1200+
pitch = MulOverflow((MulOverflow(width, bpp, overflow) + 31u) / 32u, sizeof(uint32_t), overflow);
1201+
slice = MulOverflow(pitch, height, overflow);
11561202
}
11571203
}
11581204
else
11591205
{
11601206
// Default byte alignment
1161-
pitch = (uint64_t(width) * bpp + 7u) / 8u;
1162-
slice = pitch * uint64_t(height);
1207+
pitch = (MulOverflow(width, bpp, overflow) + 7u) / 8u;
1208+
slice = MulOverflow(pitch, height, overflow);
11631209
}
11641210
}
11651211
break;
11661212
}
11671213

1214+
// A 64-bit overflow in the computations above would silently produce a slice pitch
1215+
// inconsistent with the row pitch and scanline count, which callers rely on to size
1216+
// allocations. Reject rather than truncate.
1217+
if (overflow)
1218+
{
1219+
rowPitch = slicePitch = 0;
1220+
return HRESULT_E_ARITHMETIC_OVERFLOW;
1221+
}
1222+
11681223
#if defined(_M_IX86) || defined(_M_ARM) || defined(_M_HYBRID_X86_ARM64)
11691224
static_assert(sizeof(size_t) == 4, "Not a 32-bit platform!");
11701225
if (pitch > UINT32_MAX || slice > UINT32_MAX)

0 commit comments

Comments
 (0)