Skip to content

Commit

Permalink
Allow partial chunk tables (#1653)
Browse files Browse the repository at this point in the history 
* fix sporadic UB in dwa

Signed-off-by: Kimball Thurston <[email protected]>

* Allow partial chunk tables

This will fix #1642, re-enabling partial chunk tables during
reconstruction (at least for the last part read). Correctly handles
reconstructing tables where discovered tiles are shifted from expected
indices

Signed-off-by: Kimball Thurston <[email protected]>

* Avoid OOB write

When reconstructing chunk table with corrupt chunks, avoid writing out
of bounds

Signed-off-by: Kimball Thurston <[email protected]>

* avoid undefined behavior

unaligned reads are UB, use (__builtin_) memcpy

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>
  • Loading branch information
@kdt3rd @cary-ilm
kdt3rd authored and cary-ilm committed Mar 4, 2024
1 parent efdfcff commit 58f2ab0
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 17 deletions.
46 changes: 36 additions & 10 deletions src/lib/OpenEXRCore/chunk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -454,10 +454,12 @@ reconstruct_chunk_table (
uint64_t* chunktable)
{
exr_result_t rv = EXR_ERR_SUCCESS;
exr_result_t firstfailrv = EXR_ERR_SUCCESS;
uint64_t offset_start, chunk_start, max_offset;
uint64_t* curctable;
const struct _internal_exr_part* curpart = NULL;
int found_ci, computed_ci, partnum = 0;
size_t chunkbytes;

curpart = ctxt->parts[ctxt->num_parts - 1];
offset_start = curpart->chunk_table_offset;
Expand Down Expand Up @@ -492,6 +494,11 @@ reconstruct_chunk_table (
if (rv != EXR_ERR_SUCCESS) return rv;
}

chunkbytes = (size_t)part->chunk_count * sizeof(uint64_t);
curctable = (uint64_t*) ctxt->alloc_fn (chunkbytes);
if (!curctable) return EXR_ERR_OUT_OF_MEMORY;

memset (curctable, 0, chunkbytes);
for (int ci = 0; ci < part->chunk_count; ++ci)
{
if (chunktable[ci] >= offset_start && chunktable[ci] < max_offset)
Expand All @@ -503,21 +510,37 @@ reconstruct_chunk_table (
if (part->lineorder == EXR_LINEORDER_DECREASING_Y)
computed_ci = part->chunk_count - (ci + 1);
found_ci = computed_ci;

rv = read_and_validate_chunk_leader (
ctxt, part, partnum, chunk_start, &found_ci, &offset_start);
if (rv != EXR_ERR_SUCCESS) return rv;
if (rv != EXR_ERR_SUCCESS)
{
chunk_start = 0;
if (firstfailrv == EXR_ERR_SUCCESS) firstfailrv = rv;
}

// scanlines can be more strict about the ordering
if (part->storage_mode == EXR_STORAGE_SCANLINE ||
part->storage_mode == EXR_STORAGE_DEEP_SCANLINE)
{
if (computed_ci != found_ci) return EXR_ERR_BAD_CHUNK_LEADER;
if (computed_ci != found_ci)
{
chunk_start = 0;
if (firstfailrv == EXR_ERR_SUCCESS)
firstfailrv = EXR_ERR_BAD_CHUNK_LEADER;
}
}

chunktable[found_ci] = chunk_start;
if (found_ci >= 0 && found_ci < part->chunk_count)
{
if (curctable[found_ci] == 0)
curctable[found_ci] = chunk_start;
}
}
memcpy (chunktable, curctable, chunkbytes);
ctxt->free_fn (curctable);

return rv;
return firstfailrv;
}

static exr_result_t
Expand Down Expand Up @@ -566,10 +589,9 @@ extract_chunk_table (
if (rv != EXR_ERR_SUCCESS)
{
ctxt->free_fn (ctable);
return rv;
ctable = (uint64_t *) UINTPTR_MAX;
}

if (!ctxt->disable_chunk_reconstruct)
else if (!ctxt->disable_chunk_reconstruct)
{
// could convert table all at once, but need to check if the
// file is incomplete (i.e. crashed during write and didn't
Expand All @@ -592,13 +614,17 @@ extract_chunk_table (
rv = reconstruct_chunk_table (ctxt, part, ctable);
if (rv != EXR_ERR_SUCCESS)
{
ctxt->free_fn (ctable);
ctable = (uint64_t*) UINTPTR_MAX;
if (ctxt->strict_header)
return ctxt->report_error (
{
ctxt->free_fn (ctable);
ctable = (uint64_t *) UINTPTR_MAX;
rv = ctxt->report_error (
ctxt,
EXR_ERR_BAD_CHUNK_LEADER,
"Incomplete / corrupt chunk table, unable to reconstruct");
}
else
rv = EXR_ERR_SUCCESS;
}
}
}
Expand Down
19 changes: 15 additions & 4 deletions src/lib/OpenEXRCore/internal_dwa_compressor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1699,12 +1699,23 @@ DwaCompressor_setupChannelData (DwaCompressor* me)
cd->planarUncRle[0] = cd->planarUncBuffer;
cd->planarUncRleEnd[0] = cd->planarUncRle[0];

for (int byte = 1; byte < curc->bytes_per_element; ++byte)
if (!cd->planarUncBuffer)
{
cd->planarUncRle[byte] =
cd->planarUncRle[byte - 1] + curc->width * curc->height;
for (int byte = 1; byte < curc->bytes_per_element; ++byte)
{
cd->planarUncRle[byte] = 0;
cd->planarUncRleEnd[byte] = 0;
}
}
else
{
for (int byte = 1; byte < curc->bytes_per_element; ++byte)
{
cd->planarUncRle[byte] =
cd->planarUncRle[byte - 1] + curc->width * curc->height;

cd->planarUncRleEnd[byte] = cd->planarUncRle[byte];
cd->planarUncRleEnd[byte] = cd->planarUncRle[byte];
}
}

cd->planarUncType = (exr_pixel_type_t) curc->data_type;
Expand Down
16 changes: 13 additions & 3 deletions src/lib/OpenEXRCore/internal_huf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1007,13 +1007,23 @@ readUInt (const uint8_t* b)

#ifdef __APPLE__
# include <libkern/OSByteOrder.h>
# define READ64(c) OSSwapInt64 (*(const uint64_t*) (c))
# define SWAP64(c) OSSwapInt64 (c)
#elif defined(linux)
# include <byteswap.h>
# define READ64(c) bswap_64 (*(const uint64_t*) (c))
# define SWAP64(c) bswap_64 (c)
#elif defined(_MSC_VER)
# include <stdlib.h>
# define READ64(c) _byteswap_uint64 (*(const uint64_t*) (c))
# define SWAP64(c) _byteswap_uint64 (c)
#endif

#ifdef SWAP64
static inline uint64_t READ64(const uint8_t *src)
{
uint64_t v;
// unaligned reads are UB
memcpy (&v, src, sizeof(uint64_t));
return SWAP64 (v);
}
#else
# define READ64(c) \
((uint64_t) (c)[0] << 56) | ((uint64_t) (c)[1] << 48) | \
Expand Down

0 comments on commit 58f2ab0

Please sign in to comment.