From 7185963ec8fd5d46014cb1cc80e44ed4c03d1540 Mon Sep 17 00:00:00 2001 From: Scott Schneider Date: Tue, 5 Nov 2024 08:52:58 -0800 Subject: [PATCH] Account for current pointer being outside of the buffer (#316) Summary: Pull Request resolved: https://github.com/pytorch/torchcodec/pull/316 D60461269 added `TORCH_CHECK` to catch negative values. This would catch the input argument `buf_size` being negative. However, it never caught the case of `bufferData->current` pointer being past `bufferData->size`. This is due to intricacies of C++ type rules. Both `current` and `size` are `size_t` (unsigned integers), which causes the subtraction to always be non-negative. [This example](https://www.programiz.com/online-compiler/3zokStBbD2TeP) demonstrates above well. If we only had `read` method the `current` would never be able to go past `size`, but we have `seek` method that can set `current` to arbitrary values. The diff here makes sure that we actually catch these negative values and fail with reasonable error instead of segfaulting trying to read memory outside of the buffer. Reviewed By: ahmadsharif1 Differential Revision: D65118896 --- src/torchcodec/decoders/_core/FFMPEGCommon.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp index 8d2c1c033..ab50414fc 100644 --- a/src/torchcodec/decoders/_core/FFMPEGCommon.cpp +++ b/src/torchcodec/decoders/_core/FFMPEGCommon.cpp @@ -69,6 +69,12 @@ AVIOContext* AVIOBytesContext::getAVIO() { int AVIOBytesContext::read(void* opaque, uint8_t* buf, int buf_size) { struct AVIOBufferData* bufferData = static_cast(opaque); + TORCH_CHECK( + bufferData->current <= bufferData->size, + "Tried to read outside of the buffer: current=", + bufferData->current, + ", size=", + bufferData->size); buf_size = FFMIN(buf_size, bufferData->size - bufferData->current); TORCH_CHECK( buf_size >= 0,