Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CORE-7061] sr/store: Deep copy schema_def iobufs at the interface #23114

Merged

Commits on Aug 28, 2024

  1. sr/store: Deep copy schema_def iobufs at the interface

    sharded_store's main purpose is to supply a consistent interface
    to an underlying (sharded) in-memory store. So to process a given
    request, it must first work out the home shard for the desired
    schema ID or subject, dispatching requests to that shard.
    
    Recently, schema registry was refactored to store schema definitions
    as iobufs (rather than contiguous strings) to avoid large allocations.
    To avoid semantically unnecessary _copies_, get_schema_definition was
    implemented to return a iobuf share across a shard boundary. This is
    an unsafe operation, as the reference counting and deletion machinery
    of the underlying temporary buffers is totally unsynchronized. This is
    UB in principle (data race) and leads to memory access faults (UAF, etc.)
    in practice.
    
    So this commit changes 'store::get_schema_definition' to return a copy
    of the stored schema definition (iobuf), eliminating any cross-shard
    sharing of the underlying temp buffers. This approach is rather blunt
    and carries some cost at deallocation time (seastar allocator must
    work out the appropriate site for memory reclamation for each component
    temp buffer of a given schema def), but it should be safe since the
    component temp buffers are never live on multiple shards simultaneously.
    
    Note that this path could be optimized somewhat by returning a shared
    iobuf managed by foreign_ptr and carrying out the necessary copy (a
    read-only operation) on the requesting shard.
    
    This commit also includes a "unit test" that is more of a memory access
    canary, of sorts. Reverting the change in 'store::gets_schema_definition'
    will cause the test to fail reliably, producing memory faults in line with
    the reasoning above. With the change in place, this test reliably runs to
    completion without issue.
    
    Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
    oleiman committed Aug 28, 2024
    Configuration menu
    Copy the full SHA
    420bd28 View commit details
    Browse the repository at this point in the history