Skip to content

Refuse buffers of another typesize in Python prefilters and postfilters - #773

Open
FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-callback-typesize-guard
Open

FabianIsensee wants to merge 1 commit into
Blosc:mainfrom
FabianIsensee:fix-callback-typesize-guard

Conversation

@FabianIsensee

Copy link
Copy Markdown
Contributor

general_postfilter, general_prefilter and general_filler (blosc2_ext.pyx) wrap the buffers they get from C-Blosc2 in NumPy arrays of size // typesize items of the callback's dtype. If the typesize of the buffer is smaller than the dtype's itemsize, the arrays reach past the end of the buffer and the user function writes there.

How to trigger it on main:

  • Read a vlmetalayer of an SChunk that has a postfilter. C-Blosc2 decompresses vlmetalayers with the super-chunk's postfilter (fixed in Keep prefilters, postfilters and chunk cparams away from vlmetalayer content c-blosc2#857), so with an int32 postfilter schunk.vlmeta["info"] hands the 9-byte msgpack content (typesize 1) to the callback as 9 int32 items. output[:] = input writes 36 bytes, and glibc aborts later with corrupted size vs. prev_size. schunk.to_cframe() does the same, because it copies the vlmetalayers.
  • Use an SChunk whose typesize is smaller than the callback dtype, e.g. typesize=1 with an int32 prefilter, postfilter or filler: the arrays are 4 times the buffer.
  • With a typesize larger than the itemsize (e.g. an SChunk with the default typesize 8 and an int32 callback), the arrays cover only part of each block. Half of the items are never processed, silently: they come out unprocessed for a postfilter and as zeros for a prefilter.

This PR stores the dtype's itemsize with the callback. If the buffer has another typesize, the callback raises ValueError (e.g. postfilter: data typesize 1 does not match dtype itemsize 4) before building arrays or writing anything. C-Blosc2 then stops, and the caller raises that exception after the C call, through _raise_callback_error() from #766. I checked that it arrives as is on every read and write path: decompress_chunk(), get_slice(), schunk[...], iterchunks(), vlmeta reads, to_cframe(), NDArray reads through array.schunk, append_data(), update_data(), insert_data() and schunk[...] = .... The docstrings of SChunk.postfilter(), prefilter() and filler() now say that the itemsize must be the typesize.

The check is exact, so it also refuses the larger-typesize case above, which never gave correct results. I checked the callers that install these callbacks: only SChunk.postfilter(), prefilter() and filler() do, also for NDArrays through array.schunk. Lazy expressions and UDFs use other callbacks (miniexpr_prefilter, the Menudet prefilters, general_udf_prefilter, ...), which this PR does not touch. The existing tests (int32, float32, float64, complex128, datetime64, NDArrays, concurrent reads) and the examples pass unchanged.

SChunk.filler() now removes the filler also when one of its appends fails. Before, the filler stayed set, holding only the id of its inputs tuple, which a later append would have used after the tuple was gone.

Behaviour change to be aware of: until python-blosc2 bundles a c-blosc2 with the fix linked above, reading a vlmetalayer of an SChunk that has a Python postfilter raises that ValueError. On main it returns the content but corrupts the heap. With that c-blosc2 fix it just works.

Tests, each with the GIL kept and released (blosc2.set_releasegil()):

  • test_postfilter_wrong_typesize, test_prefilter_wrong_typesize, test_filler_wrong_typesize (typesize 1 and 8 with an int32 callback): the operation raises the ValueError, the callback is never called, nothing is stored, and a failed filler is gone afterwards.
  • test_postfilter_vlmeta, from the reproducer: reading the vlmetalayer and to_cframe() either give the right content (c-blosc2 with the fix) or raise the ValueError (bundled c-blosc2). The callback never sees the content. Once a c-blosc2 with the fix is bundled, the refusal branches can go.

The callbacks in the tests only record their calls and never touch their arrays, so they cannot overrun anything even on a build without the guard. All 14 test cases fail on main, without crashing. Full suite: same results as main (only the 2 b2view tests that need aiohttp fail locally), with the bundled c-blosc2 and with the c-blosc2 fix.

Perf: one integer comparison per callback call, next to building two NumPy arrays and calling a Python function; the raise and its message are only on the refusal path. I measured a copying int32 postfilter and prefilter over 4 KiB blocks (8192 calls of ~0.85 µs; AMD EPYC 9554, 1 thread, interleaved A/B). On these µs-scale calls, builds that run the same instructions already differ by up to ~5%: inserting an unused function before general_postfilter made the build without the guard 4.8% slower. The builds with the guard stayed inside that range, on the earlier base (from -0.1% to +2.8% on the postfilter case over four runs) and on current main (postfilter -1.8%, prefilter +3.0%). No measurable change at that level.

I used Claude Code (Anthropic) to find the problem, write the guard and the tests, and run the checks and measurements.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP

general_postfilter, general_prefilter and general_filler hand Python
arrays of size // typesize items of the callback's dtype.  When the
typesize of the buffer is smaller than the dtype's itemsize, the arrays
reach past the end of the buffer and the callback writes there: C-Blosc2
before Blosc/c-blosc2#857 decompresses vlmetalayers with the
super-chunk's postfilter, so reading schunk.vlmeta[...] of an SChunk
with an int32 postfilter writes 36 bytes into a 9-byte msgpack buffer,
and glibc aborts later ("corrupted size vs. prev_size").  The same
happens with an SChunk whose typesize is smaller than the callback
dtype.  A larger typesize gives arrays that cover part of the buffer
only, so part of the result is silently left unwritten (or zero for
prefilters).

Store the itemsize with the callback and raise ValueError ("postfilter:
data typesize 1 does not match dtype itemsize 4"), before building the
arrays or writing anything, when the typesize of the buffer differs.
C-Blosc2 then stops, and the callers raise the pending exception after
the C call (_raise_callback_error(), Blosc#766).

SChunk.filler() now removes the filler also when an append fails: it
stayed set before, with only the id of its inputs tuple, which a later
append would have used after the tuple was gone.

The tests use callbacks that only record their calls, so they cannot
overrun anything even without the guard, and run with the GIL kept and
released: postfilter, prefilter and filler with typesize 1 and 8 for
int32, and the vlmeta read/to_cframe() of an SChunk with a postfilter
(with the bundled C-Blosc2 the guard refuses the vlmetalayer; with
Blosc/c-blosc2#857 the content is read without the postfilter).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DiPFztdpnKMJkzrjT36BuP

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant