Summary
Batch of five low-severity, code-verified findings from the second-pass review. Filed together to keep the tracker focused; split out any that get scheduled.
shared_seek SEEK_END: when set_filesize fails after a successful inner seek, s->inner_pos is left stale — a later cache miss can skip the corrective inner seek and cache wrong-offset data under a valid CRC
Location: libavformat/shared.c:825-838 (failure return at 835-836 without updating inner_pos), read-side trust at 686 and 738 — severity: low
In the SEEK_END branch taken when the spacemap filesize is unknown, the code physically seeks the inner protocol (res = ffurl_seek(s->inner, pos, whence), 831) and only afterwards calls set_filesize. If set_filesize fails (another process concurrently published a different size — the exact race window the set-once function exists for), it returns AVERROR(EINVAL) at 836 WITHOUT updating s->inner_pos, even though the inner protocol is now positioned at res. s->inner_pos is the sole guard deciding whether shared_read re-seeks the inner stream (686: if (s->inner_pos != inner_pos)), and the owner fetch path then asserts and trusts it (738: av_assert0(inner_pos == block_pos)). If the stale s->inner_pos happens to equal the next miss's target (e.g. both 0 right after open), the seek is skipped, ffurl_read pulls bytes from offset res instead, and the block is cached and published with a perfectly valid CRC over wrong-offset data — silent, persistent, shared corruption. Reachability is narrow with an http inner (http_seek_internal returns ENOSYS for SEEK_END when its filesize is unknown, http.c:2165-2166, and when http knows the size shared_open normally already populated the spacemap via ffurl_size), so this needs a non-http inner supporting SEEK_END without AVSEEK_SIZE, plus the concurrent size-publish race — hence low severity despite the silent-corruption outcome.
Evidence: shared.c:830-838 (verified):
res = ffurl_seek(s->inner, pos, whence);
if (res < 0)
return res;
/* Opportunistically update known filesize */
if (set_filesize(h, res - pos) < 0)
return AVERROR(EINVAL); /* inner moved, s->inner_pos NOT updated */
av_log(h, AV_LOG_DEBUG, "Inner seek to 0x%"PRIx64"\n", res);
return s->pos = s->inner_pos = res;shared.c:686-705 skips the corrective seek whenever s->inner_pos == inner_pos; 738 av_assert0(inner_pos == block_pos) then the fetch loop caches whatever the inner connection returns and publishes the CRC at 789. http.c:2165-2166 confirms the http-inner case usually errors out before the hazard.
Fix: Update s->inner_pos as soon as the inner seek succeeds, before any failure return: s->inner_pos = res; if (set_filesize(h, res - pos) < 0) return AVERROR(EINVAL); return s->pos = res;. Audit any other site where ffurl_seek(s->inner, ...) succeeds but the function then errors out.
No compile-time guarantee that the mmap'd cross-process atomics are lock-free/address-free; a non-lock-free fallback silently voids all synchronization
Location: libavformat/shared.c:106-119 (Block.state atomic_ushort, Spacemap.filesize atomic_ullong, hash atomic_uchar[32]) — severity: low
The protocol's correctness rests on C11 atomics in MAP_SHARED memory operating across processes. C11 (7.17.5) only guarantees address-free operation — the property required for cross-process atomics — for lock-free atomics. If the target's atomic_ullong (the 64-bit filesize) is not lock-free, the compiler lowers it to libatomic calls serializing through a PROCESS-LOCAL lock table: two processes then mutate filesize with no mutual exclusion or ordering at all (torn 64-bit reads possible), and the set-once CAS protocol plus the release(filesize)->release(state)/acquire(state) publication chain that tail-block clamping depends on silently stop working. Real on 32-bit targets without 64-bit CAS (older ARM, some MIPS/PPC configs FFmpeg supports) and invisible at runtime. No static assert or configure check exists anywhere in the file.
Evidence: shared.c:110-119:
typedef struct Spacemap {
atomic_uint header_magic;
atomic_ushort version;
atomic_ushort block_shift;
atomic_ullong filesize; /* byte offset of true EOF, or 0 if unknown */
atomic_uchar hash[HASH_SIZE];
...plus atomic_ushort state in Block (107), all accessed by multiple processes via the MAP_SHARED mappings created at shared.c:363/421. No occurrence of ATOMIC_*_LOCK_FREE, atomic_is_lock_free, or any static assertion in the file (verified by full read).
Fix: Enforce the precondition at compile time next to the struct definitions:
_Static_assert(ATOMIC_SHORT_LOCK_FREE == 2 && ATOMIC_INT_LOCK_FREE == 2 &&
ATOMIC_CHAR_LOCK_FREE == 2 && ATOMIC_LLONG_LOCK_FREE == 2,
"shared: requires address-free (lock-free) atomics for cross-process use");or compile the protocol out via a configure dependency when ATOMIC_LLONG_LOCK_FREE != 2.
max_packet_size/min_packet_size/short_seek all pinned to block_size: block_shift=30 (allowed) forces 1 GiB AVIOContext buffers and turns forward seeks below ~2*block_size into full sequential downloads; block_shift=9 forces 512-byte I/O
Location: libavformat/shared.c:327-328, 856-861, 868 (option range 9..30, also accepted from foreign spacemaps via 502-505); cross: avio.c:422-435, aviobuf.c:269-289 — severity: low
shared_open sets h->max_packet_size = h->min_packet_size = s->block_size (327-328). ffio_fdopen sizes the AVIOContext buffer to max_packet_size (avio.c:424-426: buffer_size = max_packet_size; /* no need to bufferize more than one packet */): at -block_shift 30 that is a 1 GiB av_malloc per opened input, fill_buffer asks shared_read for the full block, and shared_read fetches the entire block from http before returning the first byte (739-763) — multi-second first-byte latency, plus a second 1 GiB tmp_buf in non-mmap fallback (320). The block_shift can also be imposed by a foreign spacemap (502-505 accepts 9..30). Additionally shared_get_short_seek returns at least block_size (856-861) and avio_seek honors it via short_seek_get (aviobuf.c:270-272 FFMAX), so any forward seek with offset1 <= buffer_size + short_seek (up to ~2*block_size) is satisfied by looping fill_buffer (aviobuf.c:281-286) — downloading and caching up to a full block (or far more at large shifts) of unwanted data per seek. Each cold whole-block fetch also lengthens the PENDING window, magnifying the #15 timeout-race pathologies. At the other extreme, block_shift=9 makes the avio buffer 512 bytes: one shared_read (CRC over the block + atomics) per 512 bytes demuxed.
Evidence: shared.c:327-328: h->max_packet_size = s->block_size; h->min_packet_size = s->block_size;. avio.c:424-426 (verified): max_packet_size = h->max_packet_size; if (max_packet_size) { buffer_size = max_packet_size; }. shared.c:860: return ret > 0 ? FFMAX(ret, s->block_size) : s->block_size;. aviobuf.c:269-272 (verified): short_seek = ctx->short_seek_threshold; if (ctx->short_seek_get) { int tmp = ctx->short_seek_get(s->opaque); short_seek = FFMAX(tmp, short_seek); }; 281-286: forward seeks with offset1 <= buffer_size + short_seek loop fill_buffer instead of seeking.
Fix: Decouple the avio buffer from the cache block: shared_read already serves arbitrary sizes and partial blocks, so cap or drop max_packet_size (e.g. FFMIN(s->block_size, 1<<20) or 0) and stop setting min_packet_size (read-side it is only consumed by avio_write_marker). Cap shared_get_short_seek similarly (e.g. FFMIN(block_size, 256<<10)). If whole-block avio requests are desired, narrow the option range (e.g. 12..22).
shared_get_file_handle exposes the inner http socket fd although shared serves data from the cache: poll/select semantics wrong by construction
Location: libavformat/shared.c:850-854, registered at 889; contract: url.h url_get_file_handle docs; http.c:2245-2249 — severity: low
shared_get_file_handle forwards to ffurl_get_file_handle(s->inner) — for http that is the current TCP socket (http.c:2245-2249). url_get_file_handle exists so callers can poll for readability; for shared that fd is meaningless: cache-hit reads are satisfied from mmap with zero socket activity (a poller waits forever on a fully-cached file, or gets a spurious POLLHUP when the idle server closes), and conversely the socket may hold data from a previous range request that shared_read will discard via seek/reconnect. http also replaces the socket on every backward seek (http_seek_internal reconnect, 2219-2236), invalidating any fd a caller cached. In-tree consumers of ffurl_get_file_handle are protocol wrappers (rtsp/rtpproto/sapdec) unlikely to sit above shared, so the blast radius today is url-layer/out-of-tree users — but the callback as implemented can only mislead them.
Evidence: shared.c:850-854:
static int shared_get_file_handle(URLContext *h)
{
SharedContext *s = h->priv_data;
return ffurl_get_file_handle(s->inner);
}registered in ff_shared_protocol at 889. http.c:2245-2249 returns ffurl_get_file_handle(s->hd) — the live socket of the CURRENT connection.
Fix: Remove .url_get_file_handle from ff_shared_protocol (ffurl_get_file_handle then returns the documented 'not available' signal). Revisit with proper multi-handle semantics only if nonblocking support is ever implemented.
http_connect's new_location carve-out bypasses the 200-vs-206 offset check: a non-3xx response carrying a Location header lets offset-0 data be cached at block_pos with a valid CRC
Location: libavformat/http.c:1752-1761 (carve-out + offset check), 1309-1311 (parse_location on ANY response), 515-516 (redirect codes consumed elsewhere); amplification shared.c:737-789 — severity: low
http.c does detect range-ignoring servers: after the response headers, if (off != s->off) fails the request with EIO (1755-1760), because s->off is reset to 0 per request (1723) and only restored by a Content-Range header (959). That guard protects shared.c's core assumption that a successful seek+read yields bytes from block_pos. However, if (s->new_location) s->off = off; (1752-1753) forcibly satisfies the check whenever the response carries a Location header. That is correct for 3xx (the redirect branch at 515-540 follows it and the body is irrelevant), but parse_location sets new_location for ANY response containing Location (1309-1311) — a 200 with a Location header (emitted by some app servers/CDN edges as canonicalization metadata) passes the check, http_open_cnx returns success, and the body is the full entity from offset 0. shared.c then caches offset-0 bytes as block block_pos with a valid CRC (737-789): silent, persistent shared-cache corruption for all consumers, concentrated at whichever offsets get seeked to first (head/tail probe blocks).
Evidence: http.c:1752-1761 (verified):
if (s->new_location)
s->off = off;
if (off != s->off) {
av_log(h, AV_LOG_ERROR,
"Unexpected offset: expected %"PRIu64", got %"PRIu64"\n", off, s->off);
err = AVERROR(EIO);
goto done;
}parse_location (939-948) sets new_location unconditionally on a Location header; the redirect-following branch consumes it only for 301/302/303/307/308 (http.c:515-516, verified). shared.c:737-789: owner fetch trusts the successful open and publishes the CRC.
Fix: Restrict the carve-out to actual redirects: if (s->new_location && s->http_code >= 300 && s->http_code < 400) s->off = off; so a 200+Location with the wrong offset still fails the off != s->off check.
Found by second-pass multi-lens review with adversarial verification.
Summary
Batch of five low-severity, code-verified findings from the second-pass review. Filed together to keep the tracker focused; split out any that get scheduled.
shared_seek SEEK_END: when set_filesize fails after a successful inner seek, s->inner_pos is left stale — a later cache miss can skip the corrective inner seek and cache wrong-offset data under a valid CRC
Location: libavformat/shared.c:825-838 (failure return at 835-836 without updating inner_pos), read-side trust at 686 and 738 — severity: low
In the SEEK_END branch taken when the spacemap filesize is unknown, the code physically seeks the inner protocol (
res = ffurl_seek(s->inner, pos, whence), 831) and only afterwards calls set_filesize. If set_filesize fails (another process concurrently published a different size — the exact race window the set-once function exists for), it returns AVERROR(EINVAL) at 836 WITHOUT updating s->inner_pos, even though the inner protocol is now positioned at res. s->inner_pos is the sole guard deciding whether shared_read re-seeks the inner stream (686:if (s->inner_pos != inner_pos)), and the owner fetch path then asserts and trusts it (738:av_assert0(inner_pos == block_pos)). If the stale s->inner_pos happens to equal the next miss's target (e.g. both 0 right after open), the seek is skipped, ffurl_read pulls bytes from offset res instead, and the block is cached and published with a perfectly valid CRC over wrong-offset data — silent, persistent, shared corruption. Reachability is narrow with an http inner (http_seek_internal returns ENOSYS for SEEK_END when its filesize is unknown, http.c:2165-2166, and when http knows the size shared_open normally already populated the spacemap via ffurl_size), so this needs a non-http inner supporting SEEK_END without AVSEEK_SIZE, plus the concurrent size-publish race — hence low severity despite the silent-corruption outcome.Evidence: shared.c:830-838 (verified):
shared.c:686-705 skips the corrective seek whenever
s->inner_pos == inner_pos; 738av_assert0(inner_pos == block_pos)then the fetch loop caches whatever the inner connection returns and publishes the CRC at 789. http.c:2165-2166 confirms the http-inner case usually errors out before the hazard.Fix: Update s->inner_pos as soon as the inner seek succeeds, before any failure return:
s->inner_pos = res; if (set_filesize(h, res - pos) < 0) return AVERROR(EINVAL); return s->pos = res;. Audit any other site where ffurl_seek(s->inner, ...) succeeds but the function then errors out.No compile-time guarantee that the mmap'd cross-process atomics are lock-free/address-free; a non-lock-free fallback silently voids all synchronization
Location: libavformat/shared.c:106-119 (Block.state atomic_ushort, Spacemap.filesize atomic_ullong, hash atomic_uchar[32]) — severity: low
The protocol's correctness rests on C11 atomics in MAP_SHARED memory operating across processes. C11 (7.17.5) only guarantees address-free operation — the property required for cross-process atomics — for lock-free atomics. If the target's atomic_ullong (the 64-bit filesize) is not lock-free, the compiler lowers it to libatomic calls serializing through a PROCESS-LOCAL lock table: two processes then mutate filesize with no mutual exclusion or ordering at all (torn 64-bit reads possible), and the set-once CAS protocol plus the release(filesize)->release(state)/acquire(state) publication chain that tail-block clamping depends on silently stop working. Real on 32-bit targets without 64-bit CAS (older ARM, some MIPS/PPC configs FFmpeg supports) and invisible at runtime. No static assert or configure check exists anywhere in the file.
Evidence: shared.c:110-119:
plus
atomic_ushort statein Block (107), all accessed by multiple processes via the MAP_SHARED mappings created at shared.c:363/421. No occurrence of ATOMIC_*_LOCK_FREE, atomic_is_lock_free, or any static assertion in the file (verified by full read).Fix: Enforce the precondition at compile time next to the struct definitions:
or compile the protocol out via a configure dependency when ATOMIC_LLONG_LOCK_FREE != 2.
max_packet_size/min_packet_size/short_seek all pinned to block_size: block_shift=30 (allowed) forces 1 GiB AVIOContext buffers and turns forward seeks below ~2*block_size into full sequential downloads; block_shift=9 forces 512-byte I/O
Location: libavformat/shared.c:327-328, 856-861, 868 (option range 9..30, also accepted from foreign spacemaps via 502-505); cross: avio.c:422-435, aviobuf.c:269-289 — severity: low
shared_open sets h->max_packet_size = h->min_packet_size = s->block_size (327-328). ffio_fdopen sizes the AVIOContext buffer to max_packet_size (avio.c:424-426:
buffer_size = max_packet_size; /* no need to bufferize more than one packet */): at -block_shift 30 that is a 1 GiB av_malloc per opened input, fill_buffer asks shared_read for the full block, and shared_read fetches the entire block from http before returning the first byte (739-763) — multi-second first-byte latency, plus a second 1 GiB tmp_buf in non-mmap fallback (320). The block_shift can also be imposed by a foreign spacemap (502-505 accepts 9..30). Additionally shared_get_short_seek returns at least block_size (856-861) and avio_seek honors it via short_seek_get (aviobuf.c:270-272 FFMAX), so any forward seek with offset1 <= buffer_size + short_seek (up to ~2*block_size) is satisfied by looping fill_buffer (aviobuf.c:281-286) — downloading and caching up to a full block (or far more at large shifts) of unwanted data per seek. Each cold whole-block fetch also lengthens the PENDING window, magnifying the #15 timeout-race pathologies. At the other extreme, block_shift=9 makes the avio buffer 512 bytes: one shared_read (CRC over the block + atomics) per 512 bytes demuxed.Evidence: shared.c:327-328:
h->max_packet_size = s->block_size; h->min_packet_size = s->block_size;. avio.c:424-426 (verified):max_packet_size = h->max_packet_size; if (max_packet_size) { buffer_size = max_packet_size; }. shared.c:860:return ret > 0 ? FFMAX(ret, s->block_size) : s->block_size;. aviobuf.c:269-272 (verified):short_seek = ctx->short_seek_threshold; if (ctx->short_seek_get) { int tmp = ctx->short_seek_get(s->opaque); short_seek = FFMAX(tmp, short_seek); }; 281-286: forward seeks withoffset1 <= buffer_size + short_seekloop fill_buffer instead of seeking.Fix: Decouple the avio buffer from the cache block: shared_read already serves arbitrary sizes and partial blocks, so cap or drop max_packet_size (e.g. FFMIN(s->block_size, 1<<20) or 0) and stop setting min_packet_size (read-side it is only consumed by avio_write_marker). Cap shared_get_short_seek similarly (e.g. FFMIN(block_size, 256<<10)). If whole-block avio requests are desired, narrow the option range (e.g. 12..22).
shared_get_file_handle exposes the inner http socket fd although shared serves data from the cache: poll/select semantics wrong by construction
Location: libavformat/shared.c:850-854, registered at 889; contract: url.h url_get_file_handle docs; http.c:2245-2249 — severity: low
shared_get_file_handle forwards to ffurl_get_file_handle(s->inner) — for http that is the current TCP socket (http.c:2245-2249). url_get_file_handle exists so callers can poll for readability; for shared that fd is meaningless: cache-hit reads are satisfied from mmap with zero socket activity (a poller waits forever on a fully-cached file, or gets a spurious POLLHUP when the idle server closes), and conversely the socket may hold data from a previous range request that shared_read will discard via seek/reconnect. http also replaces the socket on every backward seek (http_seek_internal reconnect, 2219-2236), invalidating any fd a caller cached. In-tree consumers of ffurl_get_file_handle are protocol wrappers (rtsp/rtpproto/sapdec) unlikely to sit above shared, so the blast radius today is url-layer/out-of-tree users — but the callback as implemented can only mislead them.
Evidence: shared.c:850-854:
registered in ff_shared_protocol at 889. http.c:2245-2249 returns ffurl_get_file_handle(s->hd) — the live socket of the CURRENT connection.
Fix: Remove .url_get_file_handle from ff_shared_protocol (ffurl_get_file_handle then returns the documented 'not available' signal). Revisit with proper multi-handle semantics only if nonblocking support is ever implemented.
http_connect's new_location carve-out bypasses the 200-vs-206 offset check: a non-3xx response carrying a Location header lets offset-0 data be cached at block_pos with a valid CRC
Location: libavformat/http.c:1752-1761 (carve-out + offset check), 1309-1311 (parse_location on ANY response), 515-516 (redirect codes consumed elsewhere); amplification shared.c:737-789 — severity: low
http.c does detect range-ignoring servers: after the response headers,
if (off != s->off)fails the request with EIO (1755-1760), because s->off is reset to 0 per request (1723) and only restored by a Content-Range header (959). That guard protects shared.c's core assumption that a successful seek+read yields bytes from block_pos. However,if (s->new_location) s->off = off;(1752-1753) forcibly satisfies the check whenever the response carries a Location header. That is correct for 3xx (the redirect branch at 515-540 follows it and the body is irrelevant), but parse_location sets new_location for ANY response containing Location (1309-1311) — a 200 with a Location header (emitted by some app servers/CDN edges as canonicalization metadata) passes the check, http_open_cnx returns success, and the body is the full entity from offset 0. shared.c then caches offset-0 bytes as block block_pos with a valid CRC (737-789): silent, persistent shared-cache corruption for all consumers, concentrated at whichever offsets get seeked to first (head/tail probe blocks).Evidence: http.c:1752-1761 (verified):
parse_location (939-948) sets new_location unconditionally on a Location header; the redirect-following branch consumes it only for 301/302/303/307/308 (http.c:515-516, verified). shared.c:737-789: owner fetch trusts the successful open and publishes the CRC.
Fix: Restrict the carve-out to actual redirects:
if (s->new_location && s->http_code >= 300 && s->http_code < 400) s->off = off;so a 200+Location with the wrong offset still fails theoff != s->offcheck.Found by second-pass multi-lens review with adversarial verification.