Texture cache: fix use-after-free in async texture decode
TickAsyncDecode locked a finished decode's mutex, erased the context (freeing the mutex) and only then let the lock go out of scope, unlocking freed memory. With ASTC re-compressed on the CPU (accelerate_astc=CpuAsynchronous) title 1 hit this a few minutes in: cdb reported "Free Heap block modified after it was freed" at the mutex offset, and a run without the debugger crashed. The lock is now released before the erase. Also, deleting an image while it is being decoded (deletions other than the collector, which already skips them) cancels the decode instead of uploading into a freed slot. Verified under cdb past 330 s (failed at 243 s twice before).
This commit is contained in:
parent
9677090712
commit
5505e1ddae
2 changed files with 20 additions and 3 deletions
|
|
@ -1499,9 +1499,17 @@ void TextureCache<P>::TickAsyncDecode() {
|
|||
auto i = async_decodes.begin();
|
||||
while (i != async_decodes.end()) {
|
||||
auto* async_decode = i->get();
|
||||
std::unique_lock lock{async_decode->mutex};
|
||||
if (!async_decode->complete) {
|
||||
++i;
|
||||
{
|
||||
// Release the lock before the context is erased below: unlocking it afterwards wrote
|
||||
// into freed memory (heap corruption, crashes minutes into title 1).
|
||||
std::unique_lock lock{async_decode->mutex};
|
||||
if (!async_decode->complete) {
|
||||
++i;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
if (async_decode->cancelled) {
|
||||
i = async_decodes.erase(i);
|
||||
continue;
|
||||
}
|
||||
Image& image = slot_images[async_decode->image_id];
|
||||
|
|
@ -2452,6 +2460,14 @@ void TextureCache<P>::UntrackImage(ImageBase& image, ImageId image_id) {
|
|||
template <class P>
|
||||
void TextureCache<P>::DeleteImage(ImageId image_id, bool immediate_delete) {
|
||||
ImageBase& image = slot_images[image_id];
|
||||
if (True(image.flags & ImageFlagBits::IsDecoding)) {
|
||||
// A worker is still decoding it: keep its context alive but drop the result.
|
||||
for (auto& async_decode : async_decodes) {
|
||||
if (async_decode->image_id == image_id) {
|
||||
async_decode->cancelled = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
if (image.HasScaled()) {
|
||||
total_used_memory -= GetScaledImageSizeBytes(image);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ struct AsyncDecodeContext {
|
|||
boost::container::small_vector<BufferImageCopy, 16> copies;
|
||||
std::mutex mutex;
|
||||
std::atomic_bool complete;
|
||||
bool cancelled{}; ///< The image was deleted meanwhile: drop the result (GPU thread only).
|
||||
};
|
||||
|
||||
using TextureCacheGPUMap = ::Common::unordered_map<u64, std::vector<ImageId>, Common::IdentityHash<u64>>;
|
||||
|
|
|
|||
Loading…
Reference in a new issue